ci: harden the pipeline with lint, race tests, vulnerability, and CodeQL jobs - #44
Merged
Conversation
…eQL jobs The initial workflow ran a single serial job on ubuntu with no linter, no race detector, and no dependency scanning, and its actions were still on the deprecated Node 20 runtime. Split it into lint, test, and vulncheck jobs, add CodeQL, and add a dependabot config so the gomod and github-actions ecosystems stay current. Tests now run with -race and -shuffle=on across ubuntu, macos, and windows, since the generator is installed on all three. govulncheck deliberately runs on the stable toolchain: it reports standard-library advisories against whatever Go it runs with, and the go directive names the minimum supported release, not the one to build with.
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
errcheck flagged two unchecked MarkFlagRequired calls in generate's init. init cannot return, so the error is now joined into a package-level setupErr that Execute reports before running the command. TestGenerate_EmbeddedFields failed on windows because git checks templates out with CRLF there. The generator writes template bytes verbatim, so a windows clone emitted CRLF in every generated .go file; .gitattributes pins the working tree to LF. The test step itself failed to even start on windows: the runner defaults to PowerShell, which split -coverprofile=coverage.out into two arguments and left go test looking for a package named .out. Pin the workflow to bash.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #43, which added a deliberately minimal workflow to stop the bleeding. This brings it to what a public Go repo should actually have.
What #43 left on the table
go installed on macOS and Windows too, and nothing exercised those paths.go vet.actions/checkout@v4andactions/setup-go@v5are on the deprecated Node 20 runtime — every run emitted a deprecation annotation.Jobs
lint—gofmt -l(with a diff on failure),go mod tidydrift check,go vet, andgolangci-lint. The tidy check catches ago.mod/go.sumthat a contributor forgot to regenerate, which is otherwise invisible until it breaks someone else's build.test— matrix over ubuntu / macos / windows.go build ./..., thengo test -race -shuffle=on -coverprofile. Race matters because the generator's e2e tests spawn nestedgo testruns; shuffle catches order-dependent tests. Coverage is summarized into the job summary and uploaded as an artifact (no third-party service, no token).vulncheck—govulncheck ./....codeql—security-and-qualityqueries on PRs, pushes tocanary, and weekly, so newly published queries reach code that has already merged.Two decisions worth flagging
govulncheckruns onstable, notgo.mod's version.govulncheckreports standard-library advisories against the toolchain it runs with. Thegodirective (1.25.5) declares the minimum release this module supports, not the one to build with — pinning the scan to it would report every stdlib CVE fixed since, permanently red, for something no change to this repo can address. Scanning with the current toolchain reports what a freshgo installactually produces.Worth knowing what this does not cover: a user building with an older toolchain gets that toolchain's stdlib. That's inherent to
go installand not fixable from CI.The open Dependabot alert is not reachable.
github.com/buger/jsonparser(DoS) comes in transitively vialibopenapi.govulncheck's call-graph analysis confirms this repo doesn't reach the vulnerable code — it's in the "modules you require, but your code doesn't appear to call" bucket. The addeddependabot.yml(weekly, grouped,chore(deps)prefix so release-please classifies the commits) will pick up the fix when upstream ships it..golangci.ymlStarts conservative: the
standardset (errcheck, govet, ineffassign, staticcheck, unused) plusmisspellandusetesting, witherrcheckrelaxed in_test.go. Deliberately not a maximal linter set — a lint gate that lands with hundreds of pre-existing findings gets ignored or disabled. Easy to widen once it's green and habitual.Still needed (org-level, outside this PR)
The
parallelworksruleset on the default branch requires 2 approvals and linear history but sets norequired_status_checks. Untilci / lintandci / testare marked required, a red run still won't block a merge — which is the difference between having CI and being protected by it.Verification
gofmt,go vet,go mod tidydrift, andgo test -race -shuffle=on ./...all pass locally. The remaining jobs — golangci-lint, the macOS/Windows matrix legs, govulncheck, and CodeQL — are verified by this PR's own run.What the new jobs caught on their first run
The pipeline found three real defects immediately, all fixed in the second commit here.
1. Two unchecked errors (
errcheck,cmd/generate.go).generateCmd.MarkFlagRequired("spec")and("out")discarded their returns.initcan't return, so the errors are now joined into a package-levelsetupErrthatExecutereports before running anything — nopanic, per the repo's Go standards. Verifiedgeneratewith no flags still errors withrequired flag(s) "out", "spec" not set.2. A genuine Windows bug in the generator, not just a test failure.
TestGenerate_EmbeddedFieldsfailed onwindows-latest. Cause: git checks templates out with CRLF on Windows, and the generator writes template bytes verbatim throughtext/template— so a Windows clone emitted CRLF in every generated.gofile. Fixed at the root with.gitattributes(* text=auto eol=lf) rather than by loosening the assertion. This only affects building from a source checkout;go installpulls LF bytes from the module proxy.3. The Windows test step never even started. The runner defaults to PowerShell, which split
-coverprofile=coverage.outinto two arguments, leavinggo testlooking for a package named.out. The workflow now pinsdefaults.run.shell: bashacross the matrix.Worth noting the second one is exactly the class of bug the old single-OS pipeline could never have surfaced.
Final status
All seven checks pass:
lint,test (ubuntu-latest),test (macos-latest),test (windows-latest),vulncheck,analyze,CodeQL. CodeQL'ssecurity-and-qualitysuite reported no findings.