🌱 enable maxlength linter

[sivchari]

What this PR does / why we need it:

Part of #11834

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[sivchari]

/area api

[JoelSpeed]

Without this exclude how many exceptions are there?

[sivchari]

I met 302 exceptions.

make lint-api | grep 'maxlength' | wc -l
fatal: No names found, cannot describe anything.
INFO golangci-lint has version v1.63.4-custom-gcl built with go1.23.6 from ? on 2025-02-26 11:02:08.494916 +0000 UTC
INFO [config_reader] Used config file .golangci-kal.yml
INFO Loaded : kal
INFO [lintersdb] Active 1 linters: [kal]
INFO [loader] Go packages loading at mode 8767 (compiled_files|files|name|deps|exports_file|imports|types_sizes) took 1.189558333s
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 52.915458ms
INFO [linters_context/goanalysis] analyzers took 0s with no stages
INFO [runner] Issues before processing: 3256, after processing: 302
INFO [runner] Processors filtering stat (in/out): diff: 302/302, path_prettifier: 3256/3256, skip_files: 3256/1381, skip_dirs: 1381/1381, exclude: 1381/1381, exclude-rules: 1381/333, nolint: 333/333, max_per_file_from_linter: 302/302, severity-rules: 302/302, autogenerated_exclude: 1381/1381, identifier_marker: 1381/1381, max_from_linter: 302/302, fixer: 302/302, cgo: 3256/3256, filename_unadjuster: 3256/3256, invalid_issue: 3256/3256, uniq_by_line: 333/302, max_same_issues: 302/302, source_code: 302/302, path_shortener: 302/302, path_prefixer: 302/302, sort_results: 302/302
INFO [runner] processing took 54.777832ms with stages: identifier_marker: 18.22425ms, exclude-rules: 11.041875ms, path_prettifier: 8.284042ms, autogenerated_exclude: 6.610333ms, nolint: 5.056917ms, skip_files: 2.764125ms, source_code: 883.834µs, skip_dirs: 821.25µs, invalid_issue: 528.834µs, cgo: 266.541µs, filename_unadjuster: 225.084µs, uniq_by_line: 38.417µs, path_shortener: 23.875µs, max_per_file_from_linter: 5.75µs, max_same_issues: 916ns, fixer: 458ns, sort_results: 374ns, diff: 292ns, max_from_linter: 250ns, exclude: 250ns, severity-rules: 83ns, path_prefixer: 82ns
INFO [runner] linters took 480.364834ms with stages: goanalysis_metalinter: 425.345917ms
INFO File cache stats: 29 entries of total size 428.5KiB
INFO Memory: 19 samples, avg is 44.7MB, max is 80.6MB
INFO Execution took 1.740711833s
make: *** [lint-api] Error 1
     302

[JoelSpeed]

Not something to fix lightly then! Cool

[sbueringer]

Hm. So what is the plan with this linter?

If we ignore it only in v1beta1, what do we plan to do with v1beta2 that we cannot do today?

[JoelSpeed]

So to fix this linter, we need to add a maxLength or maxItems to strings and lists respectively.

Now, this could be seen as a backwards incompatible change, however, there are ways to mitigate that

• Choosing high enough numbers that we are confident it won't actually break anyone (be confident that it's way more than anyone could ever use)
• Rely on ratcheting validation (1.30) to enable limits and therefore, anyone who is over the limit continue to function, but won't be able to write a new value unless it's under the limit

Now, the question about moving to v1beta2. In theory, we will have to make the same trade-offs at this point, because we need to be able to convert from v1beta1 to v1beta2.
What I'm not quite sure on is whether the converted resource then goes through the v1beta2 validations or not, or whether that's only when written via the v1beta2 API.

We are probably going to have to take the hit here and say that we accept some risk in imposing length limits, or add an exception for every existing field

[chrischdi]

sgtm, better earlier then later :-)

[sivchari]

I'm in favor of first way. So I'd add the enough number for each lists.
How big a number would we like?

[sbueringer]

The problem is that we're talking about a huge number of case-by-case decisions. Which is why I was suggesting to take it over.

The alternative is probably me going over all the fields and then writing up the list of length limits we want to use, but that's a huge amount of work.

[JoelSpeed]

Agree with Stefan here, this needs case by case analysis which is quite the undertaking. I'd suggest we try to tackle this group by group, starting with the v1beta1 core types before moving through the experimental stuff

[sbueringer]

I can also do a first PR on 1-2 API types. And once we established some patterns you can follow-up for other types

[JoelSpeed]

/lgtm

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: 0202ac97101c320dbf0d3b904700c8304053eb81

[k8s-ci-robot]

New changes are detected. LGTM label has been removed.

[k8s-ci-robot]

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.