Skip to content

ci: adopt the fleet taskfile and lint baseline, and clear the debt it surfaces - #42

Merged
jacaudi merged 7 commits into
mainfrom
ci/taskfile-alignment
Aug 29, 2026
Merged

ci: adopt the fleet taskfile and lint baseline, and clear the debt it surfaces#42
jacaudi merged 7 commits into
mainfrom
ci/taskfile-alignment

Conversation

@jacaudi

@jacaudi jacaudi commented Aug 29, 2026

Copy link
Copy Markdown
Owner

Wave 0c of the CI alignment onto jacaudi/template: the non-workflow half of the copier template, plus the security debt that adopting it surfaced. Five independent commits, reviewable separately.

Rendered from the template's operator-envtest example, whose shape this repo matches exactly — variant=service, lang_go=true, has_chart=false, integration_kind=envtest. The template's own detector agrees: go=true chart=false docker=true variant=service.

Per the skill's one-PR-per-step rule this deliberately changes no workflow wiring. .github/workflows/, .github/actions/setup/, the release-please config and .copier-answers.yml all come in the follow-up workflow PR. The answers file is held back on purpose so the recorded template state never claims more is applied than actually is.

Verification

Run on the committed state, not mid-edit:

gate result
task ci exit 0
task go:integration exit 0 (envtest suite, 77s)
gosec, ratchet disabled, whole tree 0 issues
govulncheck ./... 0 vulnerabilities
yamllint 341 errors → 0
unit + envtest suites pass, both now under -race (new) — no data races

be59a85 — adopt the fleet taskfile, lint configs and gate scripts

Two configs carry repo-specific additions, marked and justified in place.

.yamllint.yml ignores config/, bundle/ and release.yaml. Same root cause the fleet baseline already documents for crds/, one directory up: controller-gen, kustomize and operator-sdk all marshal through sigs.k8s.io/yaml, whose yaml.v2 backend writes block sequences without indenting them under their key — which indent-sequences: true rejects outright. Measured before ignoring: 341 findings, 136 of them indentation inside config/ alone. Reindenting does not stick; the next make manifests undoes it. examples/ and .github/ are hand-written and were fixed instead, in 3b4b9ed.

.golangci.yml enables the migration ratchet and adds build-tags. Measured at adoption: 117 findings — 88 revive, 8 gosec, 8 nolintlint, 5 gocyclo, 3 gocritic, 2 errorlint, 2 modernize, 1 prealloc. The five gocyclo hits are the reconcile loop (Reconcile at cyclomatic complexity 121, updateWireguardPeers at 42), which #2 replaces wholesale — absorbing them by hand would mean editing code that is about to be rewritten. The 8 gosec findings were not ratcheted away; see 99d1cf4.

task smoke is rewritten, not left stock. The template's body boots the image and polls http://localhost:8080/, which cannot work here on two counts: the manager calls ctrl.GetConfigOrDie() before serving anything, so with no kubeconfig it exits 1; and its :8080 metrics listener is wrapped in WithAuthenticationAndAuthorization, so an unauthenticated GET is 401 — while the health probes are on :8081. It now asserts --help runs and prints the manager's own flags, which proves the image runs, the binary is the right architecture, and it is the manager, without needing a cluster. Left as-is it would have failed on the first release.

Also removes a committed .github/workflows/.DS_Store, and anchors the release.yaml ignore rule with a leading slash — unanchored, it matched at any depth and silently swallowed .github/workflows/release.yaml.

0b5252d — split the envtest suite out of the default test run

Tags internal/controller with //go:build integration, matching the e2e tag internal/it already carries. Three tiers, separated by what infrastructure each needs: untagged (pure unit), integration (envtest binaries), e2e (a real cluster).

ci-lint.yml runs a bare go test ./... with no KUBEBUILDER_ASSETS, and controller-runtime's test environment fails loudly rather than skipping when it cannot find them — so an untagged envtest suite turns that stage red for reasons unrelated to the change under test.

The trap in the other direction is worse and silent: a stage that builds with a tag no file carries compiles cleanly, runs zero tests, and reports success. That is why task go:integration exists rather than the suite simply being dropped from local runs.

⚠️ This created a lint-coverage regression, which is why build-tags is in be59a85. golangci-lint honours build constraints, so a tagged file is not merely unlinted — it is invisible. Without the tags the debt appeared to fall 117 → 93 when only 8 were fixed; the other 16 were hidden. With them, real remaining debt is 113, and lint now covers the internal/it e2e files that were never linted before.

3b4b9ed — make the hand-written YAML and Dockerfiles pass

Style noise mostly, but two real defects:

  • docker/setup-buildx-action@v4 was passed an input it does not have. actionlint: input "install" is not defined. It existed in earlier majors and was removed — silently ignored ever since the pin moved to v4.
  • images/test/Dockerfile installed setup-envtest at @latest (hadolint DL3062), making the image's contents depend on the day it was built and letting the test tool drift from the controller-runtime the module uses. Pinned to v0.18.5 with a renovate annotation.

31f33e0 — bump the vulnerable transitive dependencies (closes #40)

Four vulnerabilities reachable from WireguardReconciler.Reconcile:

ID module found fixed
GO-2026-6061 google.golang.org/grpc 1.58.3 1.82.1
GO-2026-5970 golang.org/x/text 0.15.0 0.39.0
GO-2026-5026 golang.org/x/net 0.25.0 0.55.0
GO-2026-4918 golang.org/x/net 0.25.0 0.53.0

The Kubernetes stack does not move. Minimal version selection permits raising an indirect above what its parent requires, so k8s.io/* v0.30.3 and controller-runtime v0.18.5 are byte-identical after this commit. Whether to bring them off the 2024-era line is left alone deliberately — not needed to clear this gate, and #40 asks for it as its own issue.

Caveat worth reading: with the modules fixed, govulncheck's remaining findings are stdlib and depend on the toolchain patch level — 7 on go1.26.4, zero on go1.26.6. CI resolves from go 1.26 via setup-go so it installs the newest 1.26.x, but that means the lint gate is sensitive to runner-image freshness: a stdlib CVE published before the image catches up turns lint red with no code change. Pinning a patch in go.mod would make it deterministic at the cost of manual bumps. Left undecided on purpose — it is an acceptance criterion on #40.

99d1cf4 — resolve every gosec finding

Not ratcheted away with the rest: a weak digest over key material, or an unbounded read on the endpoint the kubelet probes, is not the same kind of thing as a naming convention. They split three ways.

Real hardening — two listeners had no timeouts at all (G112, G114). Neither could set a ReadHeaderTimeout, the metrics one because http.ListenAndServe offers no way to. A client dribbling header bytes holds a goroutine indefinitely; enough of them and the health endpoint stops answering, which the kubelet reads as an unhealthy pod and restarts the tunnel.

A genuine upgrade — MD5 → SHA-256 (G501, G401). GetDesiredState hashes the state file for change detection. The digest is in-process only — never persisted, never compared across restarts — so the algorithm was free to change. Collision resistance is not strictly required here, but this file carries the server private key and every peer's configuration, and shipping a known-broken primitive over that material is not worth the two lines it saves.

Driven by tests, which internal/agent had none of. Five written first; TestGetDesiredStateHashIsSHA256 failed on hash length = 32, want 64 before the change. The other four pin what must survive it.

Five where the code is correct and the pattern is flagged — G304 (path is the operator's own --state flag), G301 (/dev/net at 0755 must stay world-traversable or the userspace tunnel breaks), G115 (constant device number, cannot overflow), G204 (argv from the agent's own flags; exec.Command takes argv directly, so there is no shell to inject into). Each carries its reasoning inline rather than a bare suppression.

Notes for review

Refs #37. Closes #40. Partially addresses #41.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XcHdnmhuAbyBYG2VfwuPDD

jacaudi and others added 7 commits August 28, 2026 23:05
@jacaudi @claude
ci: adopt the fleet CI taskfile, lint configs and gate scripts
be59a85 
Brings the non-workflow half of jacaudi/template (the copier template for this
fleet's pipeline) into the repo. Rendered from the `operator-envtest` example,
whose shape this repo matches exactly: variant=service, lang_go=true,
has_chart=false, integration_kind=envtest. The detector agrees — go=true,
chart=false, docker=true, variant=service.

Everything is COPIED rather than referenced. The stages that follow in the
workflow PR are local reusable workflows, so a run can never be changed by
something outside this repo's commit; `copier update` is what moves it forward.
`.copier-answers.yml` is deliberately held back until that PR, so the recorded
template state never claims more is applied than actually is.

Two configs carry repo-specific additions, both marked and justified in place:

  .yamllint.yml   ignores config/, bundle/ and release.yaml. Same root cause the
                  fleet baseline already documents for crds/, one directory up:
                  controller-gen, kustomize and operator-sdk all marshal through
                  sigs.k8s.io/yaml, whose yaml.v2 backend writes block sequences
                  without indenting them under their key, which
                  `indent-sequences: true` rejects outright. Measured before
                  ignoring: 341 findings, 136 of them indentation inside config/.
                  Reindenting does not stick — the next `make manifests` undoes
                  it. examples/ and .github/ are hand-written and were fixed
                  instead, in a later commit.

  .golangci.yml   the migration ratchet is ENABLED (`new-from-merge-base`), and
                  `build-tags` is added. Measured at adoption: 117 findings —
                  88 revive, 8 gosec, 8 nolintlint, 5 gocyclo, 3 gocritic,
                  2 errorlint, 2 modernize, 1 prealloc. The five gocyclo hits are
                  the reconcile loop (Reconcile at cyclomatic complexity 121,
                  updateWireguardPeers at 42), which #2 replaces wholesale, so
                  absorbing them by hand would mean editing code that is about to
                  be rewritten. The 8 gosec findings were NOT ratcheted away —
                  they are fixed outright, later in this branch.

`task smoke` is rewritten rather than left stock. The template's body boots the
image and polls http://localhost:8080/, which cannot work here on two counts:
the manager calls ctrl.GetConfigOrDie() before serving anything, so with no
kubeconfig it exits 1; and its :8080 metrics listener is wrapped in
WithAuthenticationAndAuthorization, so an unauthenticated GET is 401 while the
health probes are on :8081. It now asserts `--help` runs and prints the manager's
own flags, which proves the image runs, the binary is the right architecture, and
it is the manager — without needing a cluster.

Also removes .github/workflows/.DS_Store, which was committed, and anchors the
`release.yaml` ignore rule with a leading slash: unanchored, it matched at any
depth and silently swallowed .github/workflows/release.yaml.

Refs #37

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XcHdnmhuAbyBYG2VfwuPDD
@jacaudi @claude
test: split the envtest suite out of the default test run
0b5252d 
Tags internal/controller's suite with `//go:build integration`, matching the
`e2e` tag internal/it already carries. The repo now has three tiers, separated by
what infrastructure each needs:

  (untagged)         pure unit — internal/ipam, internal/iptables, internal/agent
  -tags=integration  envtest — a real kube-apiserver and etcd from downloaded
                     binaries, resolved by setup-envtest
  -tags=e2e          a real cluster

This is not cosmetic. ci-lint.yml runs a bare `go test ./...` with no envtest
assets and no KUBEBUILDER_ASSETS, and the controller-runtime test environment
FAILS LOUDLY rather than skipping when it cannot find them — which is the
behaviour you want, but it means an untagged envtest suite turns that stage red
for a reason that has nothing to do with the change under test. ci-integration.yml
is the stage that provides the assets, and it selects its work with
`-tags=integration`.

The trap in the other direction is worse and silent: a stage that builds with a
tag no file carries compiles cleanly, runs ZERO tests, and reports success. That
is why `task go:integration` exists alongside `task go:test` rather than the
suite simply being dropped from local runs — it resolves the assets itself and
runs the tagged suite, so there is still one obvious command that exercises the
controller.

Both were verified after the split: `task ci` runs the unit tier and reports
`internal/controller [no test files]`, and `task go:integration` runs the suite
to completion in 77s. Both now run under `-race`, which this repo had never done;
no data races surfaced.

Note that `build-tags` was added to .golangci.yml in the previous commit for
exactly this: golangci-lint honours build constraints, so without it a tagged
file is not merely unlinted, it is invisible — the reported debt would have
fallen by 16 findings that were hidden rather than fixed.

Refs #37, #41

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XcHdnmhuAbyBYG2VfwuPDD
@jacaudi @claude
fix: make the hand-written YAML and Dockerfiles pass the new gates
3b4b9ed 
The config/, bundle/ and release.yaml trees are generated and are exempted in
.yamllint.yml. Everything else is hand-written and is fixed here rather than
exempted — 341 yamllint findings down to zero errors.

  examples/*.yaml            missing trailing newline (2 files)
  build-images.yaml          trailing whitespace after `jobs:`, and the `steps:`
                             sequence was not indented under its key
  manual-dev-release-*.yaml  doubled space after `repository:`, and `run :` with
                             a space before the colon

Two of these turned up real defects rather than style noise:

**`docker/setup-buildx-action@v4` was being passed an input it does not have.**
actionlint: `input "install" is not defined in action
"docker/setup-buildx-action@v4"`. That input existed in earlier majors and was
removed; it has been silently ignored ever since the pin moved to v4. Removed.
Nothing depended on it — the build goes through docker/build-push-action, which
selects the buildx builder itself.

**`images/test/Dockerfile` installed setup-envtest at `@latest`.** hadolint
DL3062. An unpinned install makes the image's contents depend on the day it was
built, and lets the test tool drift away from the controller-runtime the module
actually uses. Pinned to v0.18.5, matching go.mod, with a renovate annotation so
it tracks that dependency rather than rotting.

The reindentation of build-images.yaml is whitespace only; actionlint parses both
workflows cleanly afterwards. Both files are replaced wholesale by the workflow
PR, but leaving them failing would have meant landing a taskfile whose own gates
this repo could not pass — which is the one thing this commit exists to prevent.

Refs #37

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XcHdnmhuAbyBYG2VfwuPDD
@jacaudi @claude
deps: bump the transitive dependencies carrying reachable vulnerabili…
31f33e0 
…ties

govulncheck reported four vulnerabilities reachable from
WireguardReconciler.Reconcile, via client.subResourceClient.Update and
wgtypes.GeneratePrivateKey:

  GO-2026-6061  google.golang.org/grpc  1.58.3  -> 1.82.1
  GO-2026-5970  golang.org/x/text       0.15.0  -> 0.39.0
  GO-2026-5026  golang.org/x/net        0.25.0  -> 0.55.0
  GO-2026-4918  golang.org/x/net        0.25.0  -> 0.53.0

This is pre-existing rot; the lint gate did not introduce it, it exposed it.

**The Kubernetes stack does not move.** All three modules are `// indirect`,
pulled in through k8s.io/client-go v0.30.3, sigs.k8s.io/controller-runtime
v0.18.5 and k8s.io/apiserver — but minimal version selection permits raising an
indirect above what its parent requires, so the fix is three `go get`s and a
tidy. `k8s.io/*` and controller-runtime are byte-identical after this commit.
Transitively it also raises otel 1.19 -> 1.43, protobuf 1.33 -> 1.36.11,
x/crypto, x/oauth2, x/sync, x/sys, x/term and genproto.

The separate and larger question — whether to bring client-go and
controller-runtime off the 2024-era 0.30/0.18 line — is deliberately left alone.
It is not needed to clear this gate and deserves its own change; #40 asks for it
to be filed.

Verified: go build, go vet, `task ci` and the envtest suite all pass, and
`govulncheck ./...` reports zero module vulnerabilities afterwards.

**One caveat worth recording.** With the modules fixed, govulncheck's remaining
findings come from the Go standard library and depend on the toolchain patch
level: on go1.26.4 it reports 7 (crypto/tls, net/http, net/url, encoding/asn1,
html/template, fixed in 1.26.5 and 1.26.6); on go1.26.6 it reports zero. CI
resolves its toolchain from `go 1.26` in go.mod via setup-go, so it installs the
newest 1.26.x and should see none — but that does mean the lint gate is sensitive
to the runner image's freshness, and a stdlib CVE published before the image
catches up turns lint red with no code change. Pinning a patch version in go.mod
would make it deterministic at the cost of manual bumps. Left undecided here on
purpose; it is an acceptance criterion on #40.

Closes #40
Refs #37

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XcHdnmhuAbyBYG2VfwuPDD
@jacaudi @claude
fix: resolve every gosec finding the new lint baseline reports
99d1cf4 
The fleet .golangci.yml enables gosec, which reported 8 findings. These were
deliberately NOT ratcheted away with the rest of the migration debt: a weak
digest over key material, or an unbounded read on the endpoint the kubelet
probes, is not the same kind of thing as a naming convention.

They split three ways.

**Real hardening — two listeners had no timeouts at all.**

  cmd/agent/main.go              G112  health server, `&http.Server{Addr:...}`
  internal/agent/wireguard_metrics.go  G114  metrics, `http.ListenAndServe`

Neither could set a ReadHeaderTimeout — the second because ListenAndServe offers
no way to. A client that opens a connection and dribbles header bytes holds a
goroutine indefinitely (Slowloris); enough of them and the health endpoint stops
answering, which the kubelet reads as an unhealthy pod and restarts the tunnel.
Both now use an explicit http.Server with Read/ReadHeader/Write/Idle timeouts.
The values are set independently in each rather than shared: the two endpoints
serve different traffic and are free to diverge, so this is shared shape, not
shared knowledge.

**A genuine upgrade — MD5 to SHA-256.**

  internal/agent/agent.go  G501 (import) and G401 (use)

GetDesiredState hashes the state file to detect that the desired state changed.
The digest is in-process only: computed at startup, compared against the next
read in the watcher loop, never persisted and never compared across restarts —
so the algorithm was free to change. Collision resistance is not strictly
required for change detection, but this file carries the server private key and
every peer's configuration, and shipping a known-broken primitive over that
material is not worth the two lines it saves.

Driven by tests, which internal/agent had none of. Five were written first;
TestGetDesiredStateHashIsSHA256 failed as expected on `hash length = 32, want 64`
before the change and passes after. The other four pin the properties that must
survive it — the hash is stable across reads, differs when content differs, the
state still decodes, and a missing file still errors.

**Five where the code is correct and the pattern is what is flagged.**
Each carries its reasoning inline rather than a bare suppression:

  agent.go:140       G304  path is the operator's own --state flag, a fixed mount
                           point in the pod, not attacker-controlled
  wireguard.go:89    G301  0755 on /dev/net is deliberate — it must stay
                           world-traversable to reach /dev/net/tun; 0750 breaks
                           the userspace tunnel it exists to enable
  wireguard.go:98    G115  major 10, minor 200 are compile-time constants; Mkdev
                           packs them into 0x0A000200, which fits an int
                           everywhere this builds. Nothing derives from input
  wireguard.go:113   G204  argv comes from the agent's own
                           --wg-userspace-implementation-fallback and --wg-iface
                           flags, set by the operator in the Deployment it
                           renders. exec.Command takes argv directly, so there is
                           no shell to inject into

Verified with the ratchet disabled, so the whole tree is scanned rather than only
changed lines: `golangci-lint run --new-from-merge-base="" --enable-only=gosec`
reports 0 issues. `task ci` and `task go:integration` both pass.

Refs #37

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XcHdnmhuAbyBYG2VfwuPDD
@jacaudi @claude
fix: give the linter the history its ratchet needs, and say so
b89f552 
The lint job checked out at the default depth of 1, so `origin/main` was not
present and `new-from-merge-base: origin/main` had no merge base to diff
against.

**It failed open.** golangci-lint does not error when it cannot compute the
diff — it emits

    level=warning msg="[runner] Can't process results by diff processor:
    can't prepare diff by revgrep: could not read git repo: ..."

and then reports the FULL tree: 113 pre-existing findings, on a branch whose own
changed lines were clean. The gate went red while saying nothing at all about the
change under review, and the only signal that the ratchet had stopped working was
a warning buried above the findings.

Two changes, because the fix and the trap are separate problems:

- `pull-request-workflow.yaml` now checks out with `fetch-depth: 0`. The
  template's own ci-lint.yml already does this for its own reasons, so the
  workflow PR inherits the correct behaviour — but this legacy file is what gates
  the repo until that lands.

- `.golangci.yml` documents the dependency next to the line that creates it. A
  ratchet that silently becomes a no-op is worse than no ratchet, because the
  protection is believed to be there. The comment names the exact warning string
  and tells the next person to check checkout depth before touching the config.

Verified: actionlint and yamllint clean, `task ci` exit 0.

Refs #37

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XcHdnmhuAbyBYG2VfwuPDD
@jacaudi @claude
fix: keep the envtest suite running in the Makefile test targets
7a638d7 
Tagging internal/controller with `//go:build integration` in 0b5252d silently
removed it from `make test` and `make test-ci`, which run `go test ./...` with no
tags. Both targets already export KUBEBUILDER_ASSETS, so they HAD been running
the envtest suite; afterwards they ran strictly less and still reported success.

This is the exact failure mode the commit that introduced the tag warned about,
walked into from the other side: a tag mismatch does not error, it just quietly
tests nothing. Demonstrated rather than assumed:

    $ go test -list '.*' ./internal/controller/
    ?   internal/controller  [no test files]

    $ go test -list '.*' ./internal/controller/ -tags=integration
    TestAPIs
    ok  internal/controller  0.410s

The legacy pull-request-workflow.yaml calls `make test-ci`, so without this the
envtest suite would have run nowhere in CI between this PR merging and the
workflow PR landing ci-integration.yml. Restores exactly the coverage those
targets had before the split — untagged unit tests plus the tagged suite, in one
run — and both targets now carry a comment saying why the flag is load-bearing.

Refs #37

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XcHdnmhuAbyBYG2VfwuPDD
@jacaudi
jacaudi merged commit 78c16de into main Aug 29, 2026
5 of 6 checks passed
@jacaudi
jacaudi deleted the ci/taskfile-alignment branch August 29, 2026 08:16
This was referenced Aug 29, 2026
Merged
Closed
Open
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[P0] Bump vulnerable transitive dependencies blocking the lint gate

1 participant

@jacaudi