1 Commits

Author	SHA1	Message	Date
shankar0123	3f1344e806	refactor(cmd/server): extract DI/preflight helpers to wire.go (Phase 9, 8 of N — partial) ... Phase 9 Sprint 8: shape change from the config.go cuts. cmd/server/main.go is the second-largest hotspot (2966 LOC at audit time, 2351 LOC pre-this-commit). The Phase 9 prompt asks for THREE files: main.go (entrypoint) + wire.go (DI assembly) + migrations.go (boot-time migration handling). This sprint ships TWO of those three; migrations.go is deferred with explicit rationale. Decision logged inline in wire.go's doc-comment + tasks-deferred row in the audit doc. What moved ========== cmd/server/wire.go (new, 758 lines incl. BSL header + Phase 9 doc-comment + imports + 12 declarations) Seven preflight + DI helper functions extracted from the bottom of main.go (lines 2353-2966 pre-edit): - preflightSCEPChallengePassword (H-2 fix: SCEP needs non-empty shared secret) - preflightSCEPMTLSTrustBundle (SCEP Phase 6.5: mTLS CA bundle) - preflightESTMTLSClientCATrustBundle (EST Phase 2.5: SIGHUP-reloadable *trustanchor.Holder) - preflightSCEPIntuneTrustAnchor (SCEP Phase 8.2: Intune Connector signing-cert bundle) - loadSCEPRAPair (post-preflight RA cert+key load) - preflightSCEPRACertKey (RA pair validation: mode 0600, cert/key match, NotAfter, RSA- or-ECDSA alg) - preflightEnrollmentIssuer (L-005: EST/SCEP issuer can serve GetCACertPEM) - buildFinalHandler (M-001 option D: HTTP dispatch wrapper routing auth vs no-auth chains by URL prefix) Five adapter types bridging package boundaries to avoid import cycles: - authPermissionCheckerAdapter (typed-string Authorizer → plain-string PermissionChecker) - authCheckResolverAdapter (postgres ActorRoleRepository → handler.AuthCheckResolver) - sessionMinterAdapter (session.Service → OIDC SessionMinter port) - breakglassSessionMinterAdapter (session.Service → breakglass SessionMinter + HIGH-1 revoke-all) - oidcProvidersListAdapter (postgres OIDCProviderRepository → handler.OIDCProvidersListResolver with MED-9 enabled-filter) Plus the silenceUnusedImports var-block (`_ = oidcdomain.OIDCProvider{}`) that pins the oidcdomain import as load-bearing. Why this shape rather than the full 3-file split ================================================= The Phase 9 prompt names migrations.go as the third file. The migration code in main.go is INLINE inside the 2300-line main() function — Phase 4's DEPL-M1 --migrate-only flag handling (lines ~59-77) + the RunMigrations + RunSeed + early-exit branch (lines ~199-264). It is NOT a standalone helper function ready to relocate. Extracting it into migrations.go would require: 1. Creating a new runMigrations(ctx, cfg, db, logger) error function that consolidates the inline blocks. 2. Replacing the inline code in main() with a single call site. 3. Reshaping the os.Exit(0) early-exit semantics (used at line 247 when --migrate-only is set) into a return-and-exit-from-main pattern. That's BEHAVIOR-CHANGE territory — a new function call frame, a new defer scope, error-handling pattern shift. Different shape of risk from the pure-data type relocations Sprints 1-7 did. The Phase 9 prompt explicitly says: "Do NOT change exported type signatures during the split. The refactor is mechanical relocation; behavior change is a separate concern." Creating runMigrations() doesn't change exported signatures (it'd be unexported), but the SPIRIT of the rule — "no behavior change" — is what extracting a chunk of inline code from main() into a new function pushes against (defer ordering, panic recovery, stack shape). Deferring with explicit rationale to a follow-up that the operator can review specifically for the new function-extraction risk. Estimated impact: another ~80-120 LOC out of main.go into a new migrations.go file. Recommended path: smaller standalone PR with its own review focus on the runMigrations function shape + early-exit semantics + unit tests for the new function via the existing main_test.go fixture. Imports rebalanced after the move ================================== The build surfaced 5 unused imports in main.go after the cut. Removed: - "crypto" (used only by loadSCEPRAPair return type) - "crypto/tls" (used only by preflight* X509KeyPair) - oidcdomain (used only by silenceUnusedImports; moved along with the var-block) - userdomain (used only by sessionMinterAdapter) - "github.com/certctl-io/certctl/internal/repository" (used only by adapters' EffectivePermission + OIDCProviderRepository) All five now live in wire.go's import block. Same `crypto/x509` + `encoding/pem` + `net/http` + `strings` + `time` imports that wire.go needs are STILL needed by other code in main.go, so they stay in both. Public-surface invariant ======================== All moved declarations are in package `main` (unexported by Go rules — package main cannot expose to importers). No exported surface changes. Reorganization is invisible outside cmd/server/. Same-package callers in main.go (preflight* invocations, adapter instantiation) resolve via the package symbol table. Verification (all clean): go build ./cmd/server/... → clean gofmt -l cmd/server/ → clean (after -w) staticcheck ./cmd/server/... → clean go test ./cmd/server/... -count=1 -short → ok (0.39s; existing main_test.go + preflight_*_test.go + finalhandler_test.go + auth_*_test.go + tls_test.go all pass) grep -nE '^func (preflightSCEP|preflightEST|loadSCEP|preflightEnroll|buildFinalHandler)|^type (authPermissionCheckerAdapter|authCheckResolverAdapter|sessionMinterAdapter|breakglassSessionMinterAdapter|oidcProvidersListAdapter)' cmd/server/main.go → empty (none remain in main.go) cmd/server/wire.go → 8 funcs + 5 types (correct) LOC delta: main.go: 2966 → 2347 (-619 lines: -614 from moved declarations, -5 from removed unused imports) wire.go: new, 758 lines (incl. 152-line Phase 9 doc-comment + BSL header + package decl + 16-line import block) main.go is now under 2400 LOC for the first time post-audit (audit baseline was 2966). Cumulative Phase 9 progress (all 8 sprints): config.go: 3403 → 1342 LOC (-2,061, -60.6%) across 7 sprints cmd/server/main.go: 2966 → 2347 LOC (-619, -20.9%) this sprint Pattern lesson — behavior-change boundary ========================================== Sprints 1-7 (config.go cuts) were purely mechanical relocation — data type definitions moved between sibling files in the same package. Zero risk of changing runtime semantics; the broader-importer build was the only verification needed. Sprint 8 first encountered the boundary where mechanical relocation ends. The helpers + adapter types in this sprint are still pure-mechanical (no function-call-frame change), so the bound was respected. The migrations.go extraction would cross the bound, which is why it's deferred to a dedicated review. Future sprints touching main() (Sprint 9-12 for the non-config hotspots) will face the same boundary question. The right pattern is the one this sprint demonstrated: ship the safe mechanical relocation now, defer the behavior-shift extraction with explicit rationale for the operator to review when they have time. Next queued (Sprint 9): internal/service/acme.go (1965 LOC) split into a subpackage internal/service/acme/{orders,authz,challenges, nonces,gc}.go. The current acme.go is a single-file service with related but separable concerns; the split shape here will be a NEW SUBPACKAGE rather than a sibling file, which is a third pattern (after type-family-in-sibling-file from config.go and helper-functions-in-sibling-file from this sprint). Will be the trickiest cut of Phase 9 because the import path changes from `service` (consumers do `service.ACMEService`) to `service/acme` (consumers would do `acme.Service`). Detailed planning + external- caller audit needed before any code moves. Closes: cowork/certctl-architecture-diligence-audit.html#fix-ARCH-M2 (partial — 8 of 12 — wire.go shipped; migrations.go deferred with rationale)	2026-05-14 09:02:03 +00:00