Skip to content

feat: migrate to Echo v5 - #489

Merged
retr0h merged 1 commit into
mainfrom
feat/echo-v5
Sep 12, 2026
Merged

retr0h merged 1 commit into
mainfrom
feat/echo-v5

Conversation

@retr0h

@retr0h retr0h commented Sep 12, 2026

Copy link
Copy Markdown
Collaborator

Migrates osapi from Echo v4 to Echo v5.

This was forced by #477. Dependabot's otelecho 0.69.0 → 0.71.0 bump cannot
merge: 0.71.0 marks the package deprecated, staticcheck's SA1019 fires, and
go-vet fails. The replacement, github.com/labstack/echo-opentelemetry,
instruments Echo v5, so its migration guide says plainly
that applications must migrate Echo first.

Reviewing this

Skip the 26 .gen.go files. The split is lopsided:

generated (.gen.go):     9534 insertions / 3065 deletions   26 files
hand-written + configs:   382 insertions /  329 deletions   88 files

Most of the generated churn is not Echo. The committed files were produced by
oapi-codegen v2.5.1 while go.mod has pinned v2.7.1 for some time, so
regenerating drags in two versions of accumulated output: 69 Valid() methods,
118 ContentType helpers, a bearerAuthContextKey type, and omitempty added
to four optional fields. Pre-existing drift that any regeneration surfaces.

Dependencies

Package Before After
labstack/echo v4.15.4 v5.3.1
oapi-codegen target echo-server echo5-server + strict-echo5
otelecho v0.69.0 labstack/echo-opentelemetry v0.0.3
samber/slog-echo v1.23.0 removed (see below)

go mod tidy dropped echo/v4, otelecho and slog-echo entirely. The
generator stays at the pinned v2.7.1: v2.7.1 already ships the echo5
templates
, so no bump was needed, and bumping it dragged in unrelated enum
renaming and dropped auth scopes.

Hand-written changes

  • Strict handler type. echo5-server generates StrictHandlerFunc in each
    of the 25 gen packages, so api.ScopeMiddleware cannot name any one of them.
    api.StrictHandlerFunc is declared once and the 23 call sites convert across
    the boundary, which Go permits since the signatures match.
  • Server lifecycle. v5 has no Shutdown; StartConfig.Start(ctx, e) drains
    when its context is cancelled. Both servers now hold a CancelFunc that Stop
    calls.
  • Response() returns a bare http.ResponseWriter. Status comes from
    echo.UnwrapResponse, and Flush from an http.Flusher assertion.
  • Routes() moved to Router().Routes().
  • HideBanner moved onto StartConfig.
  • CORS, below.

Two behaviour changes worth knowing

CORS is registered only when origins are configured. v5 panics on an empty
AllowOrigins where v4 silently defaulted to *, so an unconfigured deployment
would have crashed at startup rather than only in tests.

Inheriting the * was the wrong fix: it sends
Access-Control-Allow-Origin: * from an authenticated API without anyone
choosing it. Returning an error was also wrong, since CORS is optional and that
would break every deployment that never set it. So the middleware is left off,
the browser applies same-origin policy, and both outcomes are logged:

level=INFO msg="CORS not configured, cross-origin browser requests will be refused"
            configure=controller.api.security.cors.allow_origins
level=INFO msg="CORS enabled" allow_origins=[https://ui.example.com]

Nothing in internal/ or pkg/ panics, which this preserves.

slog-echo is gone, replaced by internal/telemetry/httplog. The only
v5-compatible release, v2.1.0, turns every routing error into a 500:

if _, ok := err.(*echo.HTTPError); !ok {
    err = echo.NewHTTPError(500, ...).Wrap(err)   // clobbers the real status
}

Echo v5's ErrNotFound is *echo.httpErrorunexported — so neither that
assertion nor errors.As can ever match it. Every 404 became a 500. There is no
newer release to upgrade to, so httplog uses Echo's own
RequestLoggerWithConfig with HandleError: true, which resolves the status
through the error handler that actually knows it.

It takes the same *slog.Logger built in cmd/root.go, so tint colouring,
--json, --debug and the tracing handler all pass through unchanged, and it
keeps slog-echo's request/response field groups so log consumers keep
working.

Verification

71/71 packages pass. Coverage is 100.0%, verified function-by-function
against main rather than by the headline number: the only difference is
httplog.New appearing at 100%. Nothing regressed.

Two functions had dropped and both are covered:

Function Was Now
api.Server.Stop 63.6% 100%
httplog.New 0% (new) 100%

Stop's gap was real: v5 shuts down by cancelling a context, so Stop before
Start has nothing to cancel and would have panicked on a nil CancelFunc
during a failed boot. There is now a row for that and for double-stop.

httplog's suite includes a regression test asserting an unmatched route logs
404 — confirmed to fail against slog-echo rather than pass vacuously.

The CORS change needed a behavioural test, not a coverage number. The skipped
branch has no statements, so go tool cover -func reported 100% whether or not
it ever ran, and the existing test only asserted NotNil. There is now a probe
issuing a real cross-origin request and checking the header in both directions,
verified by reintroducing the v4 behaviour:

--- FAIL: .../sends_no_CORS_header_when_no_origins_are_configured
        Error: Should be empty, but was *

Follow-up, not in here

#477 can now be closed rather than merged: otelecho is gone, so there is
nothing to bump. The bug in samber/slog-echo/v2 is still upstream and unfixed;
we are simply no longer affected.

🤖 Generated with Claude Code

https://claude.ai/code/session_01FuKUsHFG1EqZXamffh9M2c

Forced by #477. Dependabot's otelecho 0.69.0 -> 0.71.0 bump cannot merge:
0.71.0 marks the package deprecated, staticcheck's SA1019 fires and
go-vet fails. The replacement instruments Echo v5, and its migration
guide says applications must migrate Echo first.

Almost all of the echo surface is generated. Flipping 25 cfg.yaml to
echo5-server and regenerating fixed roughly 900 call sites, leaving a
small hand-written remainder:

- echo5-server generates StrictHandlerFunc in each of the 25 gen
  packages, so api.ScopeMiddleware cannot name any one of them.
  api.StrictHandlerFunc is declared once and the 23 call sites convert
  across the boundary, which Go permits since the signatures match.
- v5 has no Shutdown. StartConfig.Start drains when its context is
  cancelled, so both servers hold a CancelFunc that Stop calls.
- Response() returns a bare http.ResponseWriter: status comes from
  echo.UnwrapResponse, Flush from an http.Flusher assertion.
- Routes() moved to Router().Routes(); HideBanner moved to StartConfig.

The generator stays at the pinned v2.7.1. It already ships the echo5
templates, so no bump was needed, and bumping to v2.8.0 dragged in
unrelated enum renaming and dropped auth scopes.

slog-echo is removed rather than upgraded. Its only v5-compatible
release tests for a routing error with err.(*echo.HTTPError), and v5
returns the unexported *echo.httpError, so every 404 became a 500. There
is nothing newer to upgrade to. internal/telemetry/httplog uses Echo's
own RequestLogger with HandleError set, which resolves the status through
the error handler that knows it, and takes the same *slog.Logger from
cmd/root.go so colouring, --json and trace handling are unchanged.

CORS is registered only when origins are configured. v5 panics on an
empty AllowOrigins where v4 defaulted to "*", and neither inheriting a
permissive default nobody chose nor erroring on an optional feature is
right. The middleware is left off, the browser applies same-origin
policy, and both outcomes are logged so an operator is not left inferring
it from a browser error.

72 packages pass, coverage 100.0%, golangci-lint reports 0 issues.
Coverage was checked function by function against main rather than by the
total: the only difference is httplog.New appearing at 100%.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FuKUsHFG1EqZXamffh9M2c
@codecov

codecov Bot commented Sep 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #489   +/-   ##
=======================================
  Coverage   99.94%   99.94%           
=======================================
  Files         484      485    +1     
  Lines       22876    22940   +64     
=======================================
+ Hits        22864    22928   +64     
  Misses         12       12           
Files with missing lines Coverage Δ
internal/controller/api/agent/handler.go 100.00% <100.00%> (ø)
internal/controller/api/audit/handler.go 100.00% <100.00%> (ø)
internal/controller/api/facts/handler.go 100.00% <100.00%> (ø)
internal/controller/api/file/handler.go 100.00% <100.00%> (ø)
internal/controller/api/handler.go 100.00% <ø> (ø)
internal/controller/api/health/handler.go 100.00% <100.00%> (ø)
internal/controller/api/job/handler.go 100.00% <100.00%> (ø)
internal/controller/api/job/job_get.go 100.00% <100.00%> (ø)
internal/controller/api/middleware.go 100.00% <100.00%> (ø)
internal/controller/api/middleware_audit.go 100.00% <100.00%> (ø)
... and 21 more

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 26e9d0b...eede8d8. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@retr0h
retr0h merged commit 751d019 into main Sep 12, 2026
12 checks passed
@retr0h
retr0h deleted the feat/echo-v5 branch September 12, 2026 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant