ci: add kube-api-linter and exclude current findings by mesutoezdil · Pull Request #416 · agent-substrate/substrate

Mesut Oezdil (mesutoezdil) · 2026-07-09T17:55:41Z

Part of #207, following the plan discussed there with Julian Gutierrez Oschmann (@juli4n) and Kevin Hannon (@kannon92): plumbing plus the non invasive fixes first, with the existing invasive findings excluded so new APIs do not regress (as Kevin Hannon (@kannon92) suggested in #207 (comment)). The linter itself was originally suggested by Benjamin Elder (@BenTheElder) in #188.

Plumbing:

hack/tools/kube-api-linter module with the golangci-lint-kube-api-linter tool, same pattern as the other tools.
.golangci-kal.yaml config. The pre-existing findings from rules that need Go API changes (nomaps, nonpointerstructs, nophase, optionalfields, requiredfields, ssatags) are excluded rather than disabled, and each exclusion rule names the fields it excuses, so the rules still apply to new API files and to new fields on the existing types. Fixing them on the existing types is the follow up in Integrate with github.com/kubernetes-sigs/kube-api-linter #207.
hack/verify/kube-api-linter.sh runs it against ./pkg/api/... and is picked up by hack/verify-all.sh in CI.

Fixes in pkg/api/v1alpha1 (doc and marker only, no Go API change):

commentstart: field godocs now start with the serialized field name.
conditions: Conditions moved to the first position in ActorTemplateStatus and got the listType, listMapKey and patch markers.
defaultorrequired: SandboxConfigSpec.SandboxClass had both a default and required; now optional with omitempty, matching the same field on WorkerPoolSpec and ActorTemplateSpec. The ValidatingAdmissionPolicy in sandboxconfig-validation.yaml is unaffected since CRD defaulting runs before admission, so spec.sandboxClass is always set by then.
optionalorrequired: missing +optional markers added on ActorTemplateStatus fields.
defaults: configured preferredDefaultMarker to kubebuilder:default since CRDs are generated with controller-gen.

Generated CRDs regenerated. Structural schema changes are only the conditions listType/listMapKey and sandboxClass no longer in the required list (it is defaulted, so behavior is the same); the rest is description text.

Note: my earlier count of 50 issues in #207 was capped by the default golangci-lint issue limit. With the cap removed the real total is 86; the extra ones are requiredfields (20) and ssatags (4), both in the excluded set above.

Test plan

hack/verify/kube-api-linter.sh passes (0 issues).
Exclusions verified both ways: dropping them brings the pre-existing findings back, and a scratch field added to WorkerPoolSpec is still reported (optionalfields), which the earlier per-file exclusion swallowed.
TestSandboxConfigValidation passes against the regenerated CRD.
go build ./... and go test ./pkg/api/... ./cmd/atecontroller/... pass.
gofmt, shellcheck and the regular golangci-lint on pkg/api are clean.

Kevin Hannon (kannon92) · 2026-07-09T20:10:34Z

And sorry if my comment sends you on a wild goose chase. I am not really involved in this project but I follow it for interest.

cc Benjamin Elder (@BenTheElder) Tim Hockin (@thockin)

I think having kube-api-linter is useful btu not sure what maintainers think.

Kevin Hannon (kannon92) · 2026-07-17T18:33:13Z

Benjamin Elder (@BenTheElder)
seems like you mentioned you'd like to see this linter. Do you want to take a look?

Benjamin Elder (BenTheElder)

In the future I'd recommend:

split introducing the linter into one commit, and fixing the failure types into a commit per grouped type
prevent including merge commits

this time I think it's small enough to just squash to one commit on merge.

Tim Hockin (@thockin) Julian Gutierrez Oschmann (@juli4n) PTAL

Mesut Oezdil (mesutoezdil) · 2026-07-17T20:32:37Z

In the future I'd recommend:

split introducing the linter into one commit, and fixing the failure types into a commit per grouped type

prevent including merge commits

this time I think it's small enough to just squash to one commit on merge.

Tim Hockin (Tim Hockin (@thockin)) Julian Gutierrez Oschmann (Julian Gutierrez Oschmann (@juli4n)) PTAL

thx for feedback

Mesut Oezdil (mesutoezdil) · 2026-07-22T17:49:14Z

Haven Xia (@HavenXia) Would you mind taking a look at this one as well when you have some time?

Haven Xia (HavenXia) · 2026-07-22T22:41:37Z

Haven Xia (Haven Xia (@HavenXia)) Would you mind taking a look at this one as well when you have some time?

I don't have expertise in this area, the linter looks good to me. Benjamin Elder (@BenTheElder) can you review this and give a formal sign off?

Mesut Oezdil (mesutoezdil) · 2026-07-30T21:04:42Z

Joe Betz (@jpbetz) Benjamin Elder (@BenTheElder) Julian Gutierrez Oschmann (@juli4n) PTAL

Benjamin Elder (BenTheElder) · 2026-08-24T19:38:28Z

cc Tim Hockin (@thockin) Julian Gutierrez Oschmann (@juli4n)

Benjamin Elder (BenTheElder) · 2026-08-24T19:39:10Z

Can we squash these merge commits? As-is review is unwieldy.

Mesut Oezdil (mesutoezdil) · 2026-08-24T19:43:04Z

Benjamin Elder (@BenTheElder) thx, squashed into one commit
there are many changes for weeks :)

Benjamin Elder (BenTheElder)

I think the better way to do this is:

introduce linter with all current failures as exceptions
fix exceptions by class or type

as-is this is a rebase magnet and yet also needs pretty careful review for the API changes

Mesut Oezdil (mesutoezdil) · 2026-08-29T08:53:51Z

Done, split into two
#1314

Introduces the kube-api-linter tool (hack/tools/kube-api-linter) and .golangci-kal.yaml, wired into hack/verify-all.sh. Every finding currently produced against pkg/api/v1alpha1 is excluded precisely by field, some because the fix needs a Go API change (tracked in agent-substrate#207) and some file-wide for now (commentstart, conditions, defaultorrequired, optionalorrequired, defaults). This PR makes no content changes to the API types, so it carries no review burden beyond the tool and config themselves. Follow-up PRs remove the file-wide exclusions and fix those findings by class.

Benjamin Elder (BenTheElder)

Thanks!

Mesut Oezdil (mesutoezdil) mentioned this pull request Jul 9, 2026

Integrate with github.com/kubernetes-sigs/kube-api-linter #207

Open

Mesut Oezdil (mesutoezdil) force-pushed the feat/kube-api-linter branch from 748a99c to 4910796 Compare July 9, 2026 18:07

Kevin Hannon (kannon92) reviewed Jul 9, 2026

View reviewed changes

Comment thread .golangci-kal.yaml

Mesut Oezdil (mesutoezdil) force-pushed the feat/kube-api-linter branch 3 times, most recently from 3f964cb to 8140409 Compare July 16, 2026 06:43

Benjamin Elder (BenTheElder) assigned Julian Gutierrez Oschmann (juli4n) and Benjamin Elder (BenTheElder) Jul 17, 2026

Benjamin Elder (BenTheElder) reviewed Jul 17, 2026

View reviewed changes

Haven Xia (HavenXia) requested a review from Benjamin Elder (BenTheElder) July 22, 2026 22:41

Mesut Oezdil (mesutoezdil) force-pushed the feat/kube-api-linter branch from 4f12ca7 to 26257e5 Compare July 27, 2026 18:07

Joe Betz (jpbetz) reviewed Jul 30, 2026

View reviewed changes

Comment thread .golangci-kal.yaml

Comment thread .golangci-kal.yaml Outdated

Comment thread .golangci-kal.yaml

Comment thread pkg/api/v1alpha1/actortemplate_types.go Outdated

Comment thread manifests/ate-install/generated/ate.dev_actortemplates.yaml Outdated

Mesut Oezdil (mesutoezdil) force-pushed the feat/kube-api-linter branch from 4b73c2b to 14c2023 Compare August 24, 2026 19:42

Mesut Oezdil (mesutoezdil) force-pushed the feat/kube-api-linter branch 3 times, most recently from abe09ab to 80a338a Compare August 28, 2026 20:05

Benjamin Elder (BenTheElder) reviewed Aug 29, 2026

View reviewed changes

Mesut Oezdil (mesutoezdil) force-pushed the feat/kube-api-linter branch 2 times, most recently from c2720a6 to bc2bb03 Compare August 29, 2026 08:51

Mesut Oezdil (mesutoezdil) mentioned this pull request Aug 29, 2026

ci: fix commentstart, conditions, defaultorrequired, optionalorrequired and defaults findings in api types #1314

Open

Mesut Oezdil (mesutoezdil) force-pushed the feat/kube-api-linter branch from bc2bb03 to 856483b Compare August 29, 2026 08:58

Benjamin Elder (BenTheElder) changed the title ~~ci: add kube-api-linter and fix doc and marker findings in api types~~ ci: add kube-api-linter and exclude current findings Aug 29, 2026

Benjamin Elder (BenTheElder) approved these changes Aug 29, 2026

View reviewed changes

Benjamin Elder (BenTheElder) merged commit f936206 into agent-substrate:main Aug 29, 2026
9 checks passed

Benjamin Elder (BenTheElder) added area/api User-facing API changes area/tests Enhancing / fixing test coverage. labels Aug 29, 2026

Uh oh!

Conversation

Mesut Oezdil (mesutoezdil) commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Kevin Hannon (kannon92) commented Jul 9, 2026

Uh oh!

Uh oh!

Kevin Hannon (kannon92) commented Jul 17, 2026

Uh oh!

Benjamin Elder (BenTheElder) left a comment

Choose a reason for hiding this comment

Uh oh!

Mesut Oezdil (mesutoezdil) commented Jul 17, 2026

Uh oh!

Mesut Oezdil (mesutoezdil) commented Jul 22, 2026

Uh oh!

Haven Xia (HavenXia) commented Jul 22, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Mesut Oezdil (mesutoezdil) commented Jul 30, 2026

Uh oh!

Benjamin Elder (BenTheElder) commented Aug 24, 2026

Uh oh!

Benjamin Elder (BenTheElder) commented Aug 24, 2026

Uh oh!

Mesut Oezdil (mesutoezdil) commented Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Benjamin Elder (BenTheElder) left a comment

Choose a reason for hiding this comment

Uh oh!

Mesut Oezdil (mesutoezdil) commented Aug 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Benjamin Elder (BenTheElder) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Mesut Oezdil (mesutoezdil) commented Jul 9, 2026 •

edited

Loading

Mesut Oezdil (mesutoezdil) commented Aug 24, 2026 •

edited

Loading

Mesut Oezdil (mesutoezdil) commented Aug 29, 2026 •

edited

Loading