Part 2 ended with two next steps: a linter, then auth with mock data. Auth moves to part 4. Adding the linter turned into a dependency hygiene problem first, and fixing that properly took a step of its own.

So this part covers three things: every Go tool in its own module, a strict golangci-lint config and what it caught, and request validation against the OpenAPI spec, so a bad request gets a 422 with field errors before any handler runs and every authenticated endpoint is closed until real auth exists.

Why the tools had to move

In part 2 I pinned oapi-codegen and Air with tool directives in api/go.mod and called it done. The trade-off I accepted there was a longer go.mod. The real cost is bigger than that.

golangci-lint's install docs advise against installing it with go tool or go install, and their reasons apply to every tool:

  • A tool's dependencies can modify the dependencies of another tool or of your project.
  • The binary is built locally with your Go version, not the one the release was tested with.
  • golangci-lint's own replace directives don't apply when it's a dependency of your module.

Their fallback for people who still want go tool is a dedicated module or modfile for the linter.

The mechanism behind the first point is minimal version selection. A tool directive puts the tool's whole dependency graph into the app's module graph, and MVS picks one version per module across that graph, so the tool and the app can move each other's dependencies.

My own api/go.mod already showed the problem without the linter:

  • 42 // indirect lines, almost all of them from the two tools.
  • go mod why -m github.com/gohugoio/hugo traced a path from Air through air/runner to hugo/watcher/filenotify. My API's module graph contained Hugo.
  • make update-deps ran go get -u tool: upgrade every tool and all of their dependencies inside the API's graph. That's exactly the case the docs warn about.

Step 1: Every tool in its own module

Commit: tooling: move each Go tool into its own module

I originally planned to use alternate modfiles next to the API's go.mod, like tools/<name>.mod, selected with -modfile. It took three attempts to land somewhere else.

Attempt 1: alternate modfiles, run from api/

go mod tidy -modfile=tools/air.mod run from api/ still treats api/ as the module root, so it scans the API's packages. The result was chi, cors, envconfig and testify copied into Air's modfile, at freshly resolved versions that differed from the API's own (chi v5.4.0 against the API's v5.3.2). A tools modfile that can't be tidied from where it's used is a dead end.

Attempt 2: one shared nested module for all tools

Next, api/tools/go.mod holding all three tools. My first version reused the API's module path and had only tool lines, no requires. It needs its own module path, and go get -tool to add the requirements and a go.sum.

Once fixed, it worked: the API's go.mod was clean, regenerated code was byte-identical, and every tool ran at its pinned version. But the three tools now shared one graph, which is the same problem one level down. To measure it, I built each tool inside the shared module and on its own, then compared go version -m of the two binaries. That reports what's actually linked; go list -m all also counts test-only modules and overstates the overlap.

Tool Linked deps Built with other versions than its release resolves
oapi-codegen v2.8.0 16 6, including golang.org/x/tools v0.48.0 -> v0.50.0
Air v1.67.4 22 6, including x/sys, x/text
golangci-lint v2.14.0 207 2: spf13/cast v1.5.0 -> v1.10.0, mitchellh/mapstructure v1.5.0 -> a 2023 pseudo-version

go mod graph showed that both golangci-lint bumps came from Air through Hugo. Both libraries sit in the path golangci-lint uses to load its config. A dev reloader was changing how my linter reads .golangci.yml, and golangci-lint --version still said 2.14.0.

oapi-codegen uses x/tools to format the code it generates. The output was identical that day, but a future Air bump could change generated code in a commit that never touched oapi-codegen. That's the kind of diff nobody reviews properly, because nobody expects it.

Attempt 3: one nested module per tool

api/tools/air/, api/tools/oapi-codegen/ and api/tools/golangci-lint/, each with its own module path, go.mod and go.sum:

module github.com/davidporos92/margin-cms/api/tools/air go 1.27.1 tool github.com/air-verse/air require ( dario.cat/mergo v1.0.2 // indirect github.com/air-verse/air v1.67.4 // indirect // ... ) 

Tools run from api/ with the module selected by -modfile, so the working directory stays the API's:

CMD ["go", "tool", "-modfile=tools/air/go.mod", "air"] 

go -C tools/air tool air looks shorter, but -C changes the working directory, so Air and the linter would run on the wrong tree. And ./... doesn't descend into nested modules, so tests and lint never see the tool modules.

The result after this step: api/go.mod went from 42 indirect lines to 3, go mod why reports that the main module doesn't need Hugo, make openapi-generate-go produces byte-identical code, and Air starts in the container through the new CMD. golangci-lint's module alone is over 200 lines of go.mod. That's the graph that would otherwise have sat next to the API's.

Option Isolation Cost
tool in the app's go.mod None: tools and app share MVS Nothing extra
One shared tools module From the app, not between tools One module
One module per tool Full A longer command and one module per tool
Release binaries (golangci-lint's recommendation) Full A second install mechanism next to Go's

One mechanism with full isolation won. The cost is a longer command per tool, which lives in Make targets anyway.

make update-tools that finds tools by itself

With go get -u tool gone, the tools need their own update target. I wanted a new tool to be picked up without touching the Makefile.

My first version had cd tools/air on its own line. Make runs each recipe line in a new shell, so every go get -u tool that followed ran in api/. Two more obvious options didn't help: go list tool only warns that the pattern matched no packages in a module with no requires yet, and go get tool@latest is rejected with can't request explicit version of "tool" pattern. What works is reading the tool lines with go mod edit -json and running go get <path>@latest plus go mod tidy in each tools/*/go.mod:

TOOL_DIRS := $(patsubst %/go.mod,%,$(wildcard tools/*/go.mod)) update-tools: @for dir in $(TOOL_DIRS); do \ tools=$$(cd $$dir && go mod edit -json | sed -n '/"Tool"/,/]/s/.*"Path": "\(.*\)".*/\1@latest/p'); \ echo "Updating $$dir: $$tools"; \ (cd $$dir && go get $$tools && go mod tidy) || exit 1; \ done 

There's deliberately no -u. Each tool moves to its latest release, and its dependencies stay at what that release requires. That's golangci-lint's rule, applied to every tool, so the linter needs no special case. I checked it: after the update, golangci-lint links spf13/cast v1.5.0 and mapstructure v1.5.0 again, and a second run changes nothing.

Step 2: A strict, curated golangci-lint config

Commits: tooling: add golangci-lint with a strict config, then api: fix lint findings

golangci-lint's tool module stayed out of the step 1 commit and lands here, together with the config.

I haven't used golangci-lint in a while and wanted an up-to-date strict config, so I had Claude generate .golangci.yml. Then I went through it: I checked every key against the v2.14.0 reference config, ran golangci-lint config verify, and ran it on the code.

The shape of the config

The main decision is curated versus exhaustive. default: all with a disable list is the strictest option, but every golangci-lint upgrade can turn on new linters by itself, and a version bump suddenly fails the build for reasons unrelated to the change. So the config is default: standard plus about 40 linters listed by name, each with a one-line reason:

linters: # standard: errcheck, govet, ineffassign, staticcheck, unused. # Everything else is opt-in, so a golangci-lint upgrade never turns on new linters by itself. default: standard enable: # Bugs the compiler doesn't catch - asasalint # []any passed as a single any to a variadic func - bidichk # dangerous unicode sequences (trojan source) # ... # HTTP, context and SQL (the last two matter once Postgres lands) - bodyclose # unclosed http.Response bodies, mostly in tests # ... - rowserrcheck # rows.Err() not checked - sqlclosecheck # rows and statements not closed 

The groups are bugs, errors, HTTP/context/SQL (sqlclosecheck and rowserrcheck are there for when Postgres lands), security (gosec), logging (sloglint), tests, and style rules that prevent real confusion. Some settings go beyond the defaults:

  • govet runs every analyzer except fieldalignment.
  • errcheck treats an unchecked type assertion as an unchecked error.
  • nolintlint requires a linter name and a reason on every //nolint.
  • revive runs its defaults plus deep-exit.
  • sloglint wants the request's logger and context, not the global logger.

Generated code is excluded with generated: strict, which is the default; the config sets it explicitly because oapi-codegen's header follows the Code generated ... DO NOT EDIT. convention and the exclusion depends on it. Formatting is gofumpt and goimports with the module as the local prefix.

Left out on purpose: wsl, varnamelen, exhaustruct, mnd, lll, the complexity linters, paralleltest and err113. warn-unused flagged an exclusion rule I had for test files as unused, so it's gone until a test needs it.

Every rule that's turned off has its reason in the config:

Rule Why it's off
revive package-comments, exported Internal packages of one binary: a doc comment on every package and exported name would restate the name and add nothing
gocritic hugeParam Passing structs by value is a design choice at this size, not a defect
govet fieldalignment Reordering fields for padding hurts readability for no measurable gain

What it caught

The config and the fixes are two commits on purpose. Lint fails at the first commit, and the second shows exactly what the strict set caught. The first run found 35 issues on a codebase of stubs and one health check; with the doc-comment rules off (see the table above), the lint commit shows 25. The routine ones were 17 unused-parameter findings on the stub handlers, 10 missing doc comments and import grouping. The doc comments went away with the exported decision above, not with new comments. Renaming the stub parameters to _ then surfaced unused-receiver, fixed the same way:

func (server) ListPosts(_ context.Context, _ api.ListPostsRequestObject) (api.ListPostsResponseObject, error) { return nil, ErrNotImplemented } 

The interesting ones:

deep-exit: log.Fatal in config.New. A library function that exits the process can't be tested and skips every deferred call on the way out. config.New now returns (*Config, error) and main decides:

func New(ctx context.Context) (*Config, error) { var c Config if err := envconfig.Process(ctx, &c); err != nil { return nil, fmt.Errorf("parse config from environment: %w", err) } return &c, nil } 

The wrap also clears wrapcheck. Error wrapping has been in the standard library since Go 1.13 (%w, errors.Is, errors.As), with errors.Join and multiple %w since Go 1.20. github.com/pkg/errors, which I used last time, is in maintenance mode. What you lose is stack traces; specific wrap messages read like one.

gocritic exitAfterDefer: log.Fatal in main skips defer stop(). log.Fatal calls os.Exit, which runs no deferred calls. I switched to log.Panic: a panic in main's goroutine unwinds it and runs its deferred calls first. That only holds for the goroutine that panics. The ListenAndServe goroutine panics too, and that crashes the process without running main's defers, which is acceptable for a server that failed to bind.

gosec G706, log injection. The error handler logged r.URL.Path straight into log.Printf. Client-controlled data in a log line can fake entries (%0A in the path decodes to a newline), break log parsers or inject terminal escapes. Checking analyzers/loginjection.go in gosec revealed that the taint sources are *http.Request, *url.URL, os.Getenv and os.Args, and the sinks are every log.Print*, log.Fatal* and log.Panic*, plus the message argument of slog.Info, Warn, Error and Debug.

Quoting both r.Method and r.URL.Path fixed it:

log.Printf("%s %s %s", strconv.Quote(r.Method), strconv.Quote(r.URL.Path), err) 

The alternative was log/slog with attributes: the handler escapes attribute values, and G706 doesn't look at them. That's the better long-term answer, and it comes with the move to structured logging, which I'm planning to introduce a bit later in another post.

errchkjson: _ = json.NewEncoder(w).Encode(api.Problem{...}). Problem.Extensions is a map[string]interface{}, so the encoder can't be proven safe, and discarding its error hides a broken response. It's handled now (the full writeProblem is in step 3).

Step 3: Validate every request against the spec

The generated strict server checks types: a string where an integer belongs fails to decode. It doesn't check minLength, format, or required inside nested objects. Writing those checks in each handler duplicates the spec and drifts from it. A validator that reads the spec as data can't drift, so the spec should enforce itself at runtime, not only at compile time.

The embedded spec

Commit: api: generate the embedded spec

The validator needs the spec at runtime, and oapi-codegen can embed it (embedded-spec: true). I decided to generate it to its own file: openapi/oapi-codegen/spec.yaml generates internal/api/spec.go, so the embedded spec shows up as its own diff whenever the spec changes.

package: api generate: embedded-spec: true output: internal/api/spec.go 

kin-openapi becomes a runtime dependency of the API. Step 1 removed it with oapi-codegen; now it comes back for a real reason. I pinned it to v0.142.0, the version oapi-codegen v2.8.0 resolves in its own tool module, so the generated code and the library it calls match. With it and the validator middleware, api/go.mod is back up from 3 indirect lines to 10. Eight of them are linked into the binary; the other two come from test dependencies (testify's YAML support and a test-only dependency of kin-openapi's YAML parser). oapi-codegen v2.8.0 also generates a GetSpec() next to the older GetSwagger(); the server uses GetSpec().

Field errors get named types

Commit: spec: extract FieldError and FieldErrors schemas from Problem

Problem.errors was an inline array of objects, which generated a Go *[]struct{Field, Message string}: awkward to build in the error handler. So I added two new schemas: FieldError (one object) and FieldErrors (an array of $ref: FieldError), both registered in openapi.yaml.

Wiring the validator

Commit: api: validate requests against the spec and deny bearer auth

This commit landed after the second spec change (codes and constraints, described further down). I describe the handler first because the error codes only make sense once the error tree is clear.

The validator is nethttp-middleware v1.2.0, oapi-codegen's net/http wrapper around kin-openapi's openapi3filter. Before writing the error handler, I probed every default against the real embedded spec. Most of them weren't what I'd assumed.

  • Server matching. The spec lists http://localhost:8080/api/v1 under servers. The validator's router matches the Host too, so any other host (Docker's api:8080, production) gets 404 "no matching operation was found" for every request. The fix is DoNotValidateServers: true plus Prefix: "/api/v1". The library logs a warning about this itself, pointing to oapi-codegen issue 882.
  • format: email isn't checked unless a validator for it is registered: globally with openapi3.DefineStringFormatValidator, or per validation through SchemaValidationOptions. I used the global registry, and registered it in the constructor rather than in init(), which the linter forbids anyway.
  • A missing AuthenticationFunc already fails closed: secured operations get 401 "missing AuthenticationFunc". I added an explicit deny-all anyway. It states the intent, keeps an internal message out of responses, and is the seam for part 4.
  • Errors nest. With MultiError: true, a request with several problems arrives as MultiError -> RequestError -> MultiError -> SchemaError, and both a missing email and a missing password come back.
  • Status codes. The validator suggests 400 for bad requests. The contract says 422 (ValidationErrorResponse). With ErrorHandlerWithOpts, which I use, nethttp-middleware reports a wrong method as 404 instead of 405. The API still answers 405, because unroutable requests skip the validator and go to chi.
  • The body is read into memory twice for secured operations: once before the AuthenticationFunc runs, and again for body validation.
func newServerValidationMiddleware(router chi.Routes) (api.MiddlewareFunc, error) { oAPISpec, err := api.GetSpec() if err != nil { return nil, fmt.Errorf("get spec: %w", err) } // kin-openapi doesn't check `format: email` unless the format is explicitly registered. // The registry is package-global, so this registration applies to every spec in the process. openapi3.DefineStringFormatValidator("email", openapi3.NewRegexpFormatValidator(openapi3.FormatOfStringForEmail)) return nethttpmiddleware.OapiRequestValidatorWithOptions(oAPISpec, &nethttpmiddleware.Options{ DoNotValidateServers: true, Prefix: "/api/v1", Options: openapi3filter.Options{ MultiError: true, AuthenticationFunc: authFunc, }, ErrorHandlerWithOpts: errHandler, // Let chi answer requests which we cannot route // This will automatically set Allow for 405 Skipper: func(r *http.Request) bool { return !router.Match(chi.NewRouteContext(), r.Method, r.URL.Path) }, }), nil } func authFunc(context.Context, *openapi3filter.AuthenticationInput) error { return ErrUnauthenticated } 

Closed by default is the point of authFunc. Leaving the bearer endpoints open until part 4 would mean every new endpoint is public until someone remembers to protect it. With deny-all, part 4 opens endpoints one at a time by verifying a token, and forgetting something fails safe.

Where it's mounted

There were three candidate places for the validator:

Where Problem
Strict middlewares They run after the body is decoded into typed request objects, too late
chi's Middlewares option Generated code runs these per operation, after routing and parameter parsing, so ?limit=abc gets the generated plain-text 400 and unknown routes get chi's plain-text 404/405
Around the whole router Sees every request first

So it wraps the whole router: cors(maxBytes(validator(chi))). CORS stays outermost so preflight OPTIONS requests, which aren't in the spec, still work. The body limit sits outside the validator so the validator's own read is capped too. All of this wiring moved from main.go into server.New, which the tests can now call:

func New(cfg *Config) (*http.Server, error) { router := chi.NewRouter() router.NotFound(func(w http.ResponseWriter, _ *http.Request) { writeProblem(w, api.Problem{Status: http.StatusNotFound}) }) serverInterface := api.NewStrictHandlerWithOptions(server{}, newServerMiddlewares(), newServerOptions()) handler := api.HandlerWithOptions(serverInterface, api.ChiServerOptions{ BaseURL: "/api/v1", BaseRouter: router, }) serverValidationMiddleware, err := newServerValidationMiddleware(router) if err != nil { return nil, fmt.Errorf("failed to initialize server validation middleware: %w", err) } handler = maxBytesReaderMiddleware(cfg)(serverValidationMiddleware(handler)) handler = cors.Handler(cors.Options{ // ... })(handler) // ... } 

The error handler

There are three kinds of failure. No matched route is a 404 or 405. A security failure anywhere in the tree is a 401 with WWW-Authenticate: Bearer. Schema problems are a 422 with one field error each. Anything unrecognised is logged and answered with a 500, and err.Error() never reaches the client.

var result validationResult result.collect(err) switch { case result.unauthenticated: // Check unauthenticated first: // Unauthenticated caller doesn't get to learn what's wrong in a request. w.Header().Set("WWW-Authenticate", "Bearer") writeProblem(w, api.Problem{Status: http.StatusUnauthorized}) case result.tooLarge: writeProblem(w, api.Problem{Status: http.StatusRequestEntityTooLarge}) case result.unknown != nil: log.Printf("validate %s %s: %v", strconv.Quote(r.Method), strconv.Quote(r.URL.Path), result.unknown) writeProblem(w, api.Problem{Status: http.StatusInternalServerError}) default: writeProblem(w, api.Problem{ Status: http.StatusUnprocessableEntity, Errors: &result.fields, }) } 

Auth is checked first, so an anonymous GET /posts?limit=abc is a 401, not a 422: a caller without a token doesn't learn what else is wrong with the request.

The tree walk uses a type switch, not errors.As, and that's deliberate:

// collect walks kin-openapi's error tree. With MultiError on, a request with // several problems arrives as MultiError -> RequestError -> MultiError -> SchemaError. // // It switches on concrete types on purpose: MultiError implements As, so // errors.As on it returns only the first match and drops the rest. func (v *validationResult) collect(err error) { //nolint:errorlint // see above, errors.As would flatten MultiError switch e := err.(type) { case openapi3.MultiError: for _, inner := range e { v.collect(inner) } case *openapi3filter.SecurityRequirementsError: v.unauthenticated = true case *openapi3filter.RequestError: v.collectRequestError(e) default: v.unknown = errors.Join(v.unknown, err) } } 

kin-openapi's MultiError implements As, so errors.As returns the first match and drops the rest. Worse, errors.As(requestErr, &multi) unwraps past the RequestError and loses the parameter name with it. I hit exactly this while iterating: I replaced the switch with errors.As to satisfy errorlint, and every schema error became a 500, because the bare SchemaErrors matched nothing and fell through to unknown. Build, vet and lint all passed. The strict linter had pushed a deliberate type switch into a bug, so the switch is back, with //nolint:errorlint and a reason, which is exactly the case nolintlint's required reason is for.

writeProblem takes the whole api.Problem. Type defaults to about:blank and Title to Go's status text. With about:blank, RFC 9457 wants the title to be the status code's recommended phrase, not a custom one. Go's http.StatusText still uses the older names for two codes this server sends: "Request Entity Too Large" and "Unprocessable Entity", where RFC 9110 now says "Content Too Large" and "Unprocessable Content".

func writeProblem(w http.ResponseWriter, problem api.Problem) { if problem.Type == "" { problem.Type = "about:blank" } if problem.Title == "" { problem.Title = http.StatusText(problem.Status) } w.Header().Set("Content-Type", "application/problem+json") w.WriteHeader(problem.Status) err := json.NewEncoder(w).Encode(problem) ifSource:https://dev.to/davidporos92/building-margincms-part-3-tools-out-of-gomod-a-strict-linter-and-a-validated-edge-47n9

Related posts