Extend maxlength linter to support kubernetes declarative validation markers by itzPranshul · Pull Request #245 · kubernetes-sigs/kube-api-linter

itzPranshul · 2026-05-06T06:49:39Z

This PR extends the maxlength linter to support kubernetes DV markers:

+k8s:maxLength for string types
+k8s:maxBytes for []byte types (aligning with DV byte counting semantics)
+k8s:maxItems for slices and arrays
+k8s:maxProperties for map[string]V types

Key chages:

Added support for +k8s:enum and +k8s:format to exempt string fields from requiring maximum lengths.
Introduced MaxLengthConfig to specify whether the linter should prefer kubebuilder or k8s DV markers
Upgraded the analyzer to struct-based implementation NewConfigurableInitializer.
added dedicated tests to validate all new DV marker scenarios.

Fixes #223

k8s-ci-robot · 2026-05-06T06:49:46Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: itzPranshul
Once this PR has been reviewed and has the lgtm label, please assign jpbetz for approval. For more information see the Code Review Process.

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

Details

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 · 2026-05-23T13:44:11Z

Why do you edit this file?

Those were accidental changes introduced by my local environment while addressing the lint issues.I have reverted them. Sorry about that!

sivchari · 2026-05-23T13:45:52Z

+
+// TestMaxLength_DVMarkers tests that the linter (default config) accepts DV
+// markers as satisfying the max-length/max-items/max-properties constraints.
+func TestMaxLength_DVMarkers(t *testing.T) {


This test looks duplicating TestMaxLength. Why did you add this test ?

I've removed it now

sivchari · 2026-05-23T13:48:10Z

+	ci, ok := maxlength.Initializer().(initializer.ConfigurableAnalyzerInitializer)
+	if !ok {
+		t.Fatal("maxlength.Initializer() does not implement ConfigurableAnalyzerInitializer")
+	}
+
+	a, err := ci.Init(&maxlength.MaxLengthConfig{
+		PreferredMaxLengthMarker:     markers.K8sMaxLengthMarker,
+		PreferredMaxItemsMarker:      markers.K8sMaxItemsMarker,
+		PreferredMaxPropertiesMarker: markers.K8sMaxPropertiesMarker,
+	})


Suggested change

ci, ok := maxlength.Initializer().(initializer.ConfigurableAnalyzerInitializer)

if !ok {

t.Fatal("maxlength.Initializer() does not implement ConfigurableAnalyzerInitializer")

}

a, err := ci.Init(&maxlength.MaxLengthConfig{

PreferredMaxLengthMarker: markers.K8sMaxLengthMarker,

PreferredMaxItemsMarker: markers.K8sMaxItemsMarker,

PreferredMaxPropertiesMarker: markers.K8sMaxPropertiesMarker,

})

ci, err := maxlength.Initializer().Initialize(&maxlength.MaxLengthConfig{

PreferredMaxLengthMarker: markers.K8sMaxLengthMarker,

PreferredMaxItemsMarker: markers.K8sMaxItemsMarker,

PreferredMaxPropertiesMarker: markers.K8sMaxPropertiesMarker,

})

I like this style.

sivchari · 2026-05-23T13:53:41Z

+	// Run only against c.go-style testdata when DV markers are preferred.
+	// The want-comments in a.go cite kubebuilder markers; for this run we use a
+	// separate package that has no want-comments (happy-path only).
+	// For now we verify the analyzer initialises and runs without panicking.
+	_ = a
+	_ = testdata


Why don't you test dv-style marker?
If you don't test dv-style, we can't confirm this implementation works correctly.

I've Added a new testdata/src/b/b.go with the analyzer initialized in DV preferred mode.

DV-style markers are now tested across two layers:

Acceptance in default (Kubebuilder-preferred) config - verifies that +k8s markers suppress diagnostics even when the linter is configured to prefer kubebuilder markers.
2.End to end DV-preferred config

All tests pass. Thanks for the review!

JoelSpeed · 2026-07-03T14:00:59Z

+	MapWithNoMaxProperties map[string]string // want `field DVMaxLength.MapWithNoMaxProperties must have a maximum properties, add kubebuilder:validation:MaxProperties marker`
+
+	// Non-string-keyed map — should NOT lint.
+	// DV +k8s:maxProperties only supports string-keyed maps; linter mirrors this constraint.


What about +kubebuilder:validation:maxProperties?

The restriction actually applies to both. I've updated the comments to be generic so it's clear why it's skipped in both modes.

JoelSpeed · 2026-07-03T14:01:39Z

+	// +k8s:maxLength (counts chars) is not the correct DV tag for []byte;
+	// +k8s:maxBytes (counts bytes) should be used instead.


What's the parsing doing for these comments, they look like markers? Maybe remove the + at the beginning?

JoelSpeed · 2026-07-03T14:06:04Z

+	// DV +k8s:maxProperties only supports string-keyed maps.
+	// Skip non-string-keyed maps (e.g. map[int]string) to avoid false positives.
+	if keyIdent, ok := mapType.Key.(*ast.Ident); !ok || keyIdent.Name != "string" {
+		return
+	}


Won't this break CRD use cases? Or are non string keyed maps not possible in CRDs?

As far as I understood, non-string-keyed maps are not valid CRD fields. So, this will not break CRD use case

JoelSpeed · 2026-07-03T14:07:07Z

+// needsByteSliceMaxLength is like needsStringMaxLength but enforces that for DV markers,
+// +k8s:maxBytes is used instead of +k8s:maxLength (which counts characters).


How does this work for CRDs though where there is no byte length check, it is always character based?

For CRDs, the linter remains character-based and only-handles byte based validation +k8s:maxBytes when explicitly configured for DV.

OpenAPI v3 does not have native byte-length constraints; instead, byte[] fields are represented as base64-encoded json strings, so the linter expects +kubebuilder :validation:MaxLength for []byte fields for CRDs

kubernetes-prow · 2026-07-05T18:01:31Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: itzPranshul
Once this PR has been reviewed and has the lgtm label, please assign jpbetz for approval. For more information see the Code Review Process.

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

Details

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

…figurability

k8s-ci-robot requested review from JoelSpeed and everettraven May 6, 2026 06:49

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels May 6, 2026

itzPranshul force-pushed the extend-maxlength branch 2 times, most recently from 0730e76 to 92b1b98 Compare May 6, 2026 07:36

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 18, 2026

sivchari reviewed May 23, 2026

View reviewed changes

itzPranshul force-pushed the extend-maxlength branch from 92b1b98 to 615ec6e Compare May 24, 2026 18:59

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 24, 2026

itzPranshul mentioned this pull request May 27, 2026

REQUEST: New membership for itzPranshul kubernetes/org#6391

Closed

11 tasks

JoelSpeed reviewed Jul 3, 2026

View reviewed changes

kubernetes-prow Bot added the do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. label Jul 5, 2026

maxLength: add support for k8s declarative validation markers and con…

77257d4

…figurability

itzPranshul force-pushed the extend-maxlength branch from f0c8801 to 77257d4 Compare July 5, 2026 18:14

kubernetes-prow Bot removed the do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. label Jul 5, 2026

		// +k8s:maxLength (counts chars) is not the correct DV tag for []byte;
		// +k8s:maxBytes (counts bytes) should be used instead.

		// needsByteSliceMaxLength is like needsStringMaxLength but enforces that for DV markers,
		// +k8s:maxBytes is used instead of +k8s:maxLength (which counts characters).

Uh oh!

Conversation

itzPranshul commented May 6, 2026

Uh oh!

k8s-ci-robot commented May 6, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kubernetes-prow Bot commented Jul 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants