Add markerscope linter #161

nayuta723 · 2025-10-26T07:59:38Z

Fixes #148

Implements a new markerscope linter that validates markers are applied in the correct scope (field vs type) and to compatible data types, ensuring proper usage of kubebuilder/controller-runtime annotations throughout Kubernetes API definitions.

Key Features:

Scope Validation: Ensures markers are placed on the correct Go language construct
- FieldScope: Can only be applied to struct fields (e.g., optional, required, nullable)
- TypeScope: Can only be applied to type definitions (e.g., kubebuilder:validation:items:ExactlyOneOf)
- AnyScope: Can be applied to either fields or type definitions (e.g., Minimum, Pattern)
- Note: CRD-level markers (e.g., kubebuilder:resource, kubebuilder:subresource) and package-level markers (e.g., groupName) are out of scope for this PR
Type Constraint Validation: Ensures markers are applied to compatible OpenAPI schema types
- Numeric markers (Minimum, Maximum, MultipleOf) only for integer/number types
- String markers (Pattern, MinLength, MaxLength) only for string types
- Array markers (MinItems, MaxItems, UniqueItems) only for array types
- Object markers (MinProperties, MaxProperties) only for object types (struct/map)
- Array items markers (items:Minimum, items:Pattern, etc.) apply constraints to array element types
Strict Type Constraint Mode: strictTypeConstraint flag enforces that markers with AnyScope and type constraints must be declared on type definitions rather than fields when using named types, promoting consistent marker placement
Automatic Fix Suggestions: When policy: SuggestFix is configured, provides automatic fix suggestions for violations:
- Move markers from incorrect scope (field ↔ type)
- Remove markers with incompatible type constraints
- Relocate markers to type definitions for named types
Configurable Rules: Supports overriding default rules and adding custom marker validation
- Override built-in marker rules
- Define custom markers with scope and type constraints
- Configure dangerous type handling (float32/float64)
Comprehensive Default Rules: Includes 100+ built-in rules for:
- All standard kubebuilder validation markers
- k8s declarative validation markers
- Controller-runtime markers

k8s-ci-robot · 2025-10-26T07:59:44Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: nayuta723
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.

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

JoelSpeed

Some of this code appears to be a little over complex for what we were expecting here. Lets try and get this cleaned up and simplified a bit.

Lets focus initially on what the public configuration should look like, and then look at the implementation I think

Out of interest, were any code assistants used to help prepare this PR? (I'm currently comparing different models so good to have context on what models are being used where)

pkg/markers/markers.go

pkg/analysis/utils/utils.go

JoelSpeed · 2025-10-28T16:12:28Z

pkg/analysis/markerscope/schema.go

+// SchemaType represents OpenAPI schema types that markers can target.
+type SchemaType string
+
+const (
+	// SchemaTypeInteger represents integer types (int, int32, int64, uint, etc.)
+	SchemaTypeInteger SchemaType = "integer"
+	// SchemaTypeNumber represents floating-point types (float32, float64).
+	SchemaTypeNumber SchemaType = "number"
+	// SchemaTypeString represents string types.
+	SchemaTypeString SchemaType = "string"
+	// SchemaTypeBoolean represents boolean types.
+	SchemaTypeBoolean SchemaType = "boolean"
+	// SchemaTypeArray represents array/slice types.
+	SchemaTypeArray SchemaType = "array"
+	// SchemaTypeObject represents struct/map types.
+	SchemaTypeObject SchemaType = "object"
+)


Does this all need to be exported? Is this part of configuration?

This is configured this way because it's necessary for the Configuration settings.

pkg/analysis/markerscope/analyzer_test.go

JoelSpeed · 2025-10-28T16:15:34Z

pkg/analysis/markerscope/analyzer_test.go

+	"golang.org/x/tools/go/analysis/analysistest"
+)
+
+func TestAnalyzerWarnOnly(t *testing.T) {


We normally have a test for the default configuration, what is the default? Is defaulting of the config implemented?

The following is the default configuration. I will add a defaultConfig function.

Policy: Warn

OverrideMarkers: empty (use built-in defaults)

CustomMarkers: empty

JoelSpeed · 2025-10-28T16:58:15Z

pkg/analysis/markerscope/analyzer.go

+	// Check if dangerous types are disallowed
+	if !allowDangerousTypes && schemaType == SchemaTypeNumber {
+		// Get the underlying type for better error messages
+		underlyingType := getUnderlyingType(t)
+		return &DengerousTypeError{Type: underlyingType.String()}
+	}


I don't understand what this is trying to achieve, can you explain this?

When DangerousTypes is enabled, it allows floats configuration. However, since there is a nofloat linter, this is no longer necessary and should be removed.
https://github.com/kubernetes-sigs/kube-api-linter/pull/161/files#r2470349926

The original intention was to produce errors for patterns where SchemaTypeNumber (which includes float32, float64) is used when Float is not allowed (DangerousTypes disabled).

JoelSpeed · 2025-10-28T16:59:40Z

docs/linters.md

+lintersConfig:
+  markerscope:
+    policy: Warn | SuggestFix # The policy for marker scope violations. Defaults to `Warn`.
+    allowDangerousTypes: false # Allow dangerous number types (float32, float64). Defaults to `false`.


I don't think this linter should care about that, we have a nofloats linter already

Understood. I'm going to turn this setting off.

We handled it here.
875b36c

docs/linters.md

sivchari · 2025-10-29T05:18:53Z

pkg/analysis/markerscope/analyzer_test.go

Let's add override test.

I have taken care of the matter below.
6270f45

pkg/analysis/markerscope/analyzer.go

sivchari · 2025-10-29T07:25:53Z

pkg/analysis/markerscope/analyzer.go

+	if rule.Scope == FieldScope {
+		message = fmt.Sprintf("marker %q can only be applied to fields", marker.Identifier)
+
+		if a.policy == MarkerScopePolicySuggestFix {
+			fixes = a.suggestMoveToField(pass, typeSpec, marker, rule)
+		}
+	} else {
+		message = fmt.Sprintf("marker %q cannot be applied to types", marker.Identifier)
+	}


Suggested change

if rule.Scope == FieldScope {

message = fmt.Sprintf("marker %q can only be applied to fields", marker.Identifier)

if a.policy == MarkerScopePolicySuggestFix {

fixes = a.suggestMoveToField(pass, typeSpec, marker, rule)

}

} else {

message = fmt.Sprintf("marker %q cannot be applied to types", marker.Identifier)

}

message := fmt.Sprintf("marker %q cannot be applied to types", marker.Identifier)

if rule.Scope == FieldScope {

message = fmt.Sprintf("marker %q can only be applied to fields", marker.Identifier)

if a.policy == MarkerScopePolicySuggestFix {

fixes = a.suggestMoveToField(pass, typeSpec, marker, rule)

}

I have taken care of the matter below.
9c0a9b6

pkg/analysis/markerscope/analyzer.go

Signed-off-by: nayuta-ai <nayuta723@gmail.com>

…ag and updating type constraints. Refactor schema type definitions and improve error handling for invalid markers. Add new test cases for various marker scenarios. Signed-off-by: nayuta-ai <nayuta723@gmail.com>

…tom error types for invalid schema types, type constraints, and element constraints. Update validation functions to return these new error types for improved clarity and maintainability. Signed-off-by: nayuta-ai <nayuta723@gmail.com>

Signed-off-by: nayuta-ai <nayuta723@gmail.com>

…object lists Signed-off-by: nayuta-ai <nayuta723@gmail.com>

…h validation Signed-off-by: nayuta-ai <nayuta723@gmail.com>

Signed-off-by: nayuta-ai <nayuta723@gmail.com>

…straint and improve code quality Signed-off-by: nayuta-ai <nayuta723@gmail.com>

… constants Signed-off-by: nayuta-ai <nayuta723@gmail.com>

Signed-off-by: nayuta-ai <nayuta723@gmail.com>

nayuta723 · 2025-10-30T09:22:27Z

@JoelSpeed @sivchari
Thank you for your review. I have completed the revisions to the relevant sections. Please review them.

Out of interest, were any code assistants used to help prepare this PR? (I'm currently comparing different models so good to have context on what models are being used where)

I'm currently using Claude Sonnet 4.5 to write docstrings and handle minor implementation details.

…tive Signed-off-by: nayuta-ai <nayuta723@gmail.com>

k8s-ci-robot requested a review from everettraven October 26, 2025 07:59

k8s-ci-robot requested a review from JoelSpeed October 26, 2025 07:59

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Oct 26, 2025

JoelSpeed reviewed Oct 28, 2025

View reviewed changes

sivchari reviewed Oct 29, 2025

View reviewed changes

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

nayuta723 added 16 commits October 30, 2025 17:50

initial commit for markerscope linter

8e40e87

Signed-off-by: nayuta-ai <nayuta723@gmail.com>

Add ScopeViolation

dbf1254

Signed-off-by: nayuta-ai <nayuta723@gmail.com>

Add TypeViolation

cb5218f

Signed-off-by: nayuta-ai <nayuta723@gmail.com>

add custom marker rule setting

c096c8f

Signed-off-by: nayuta-ai <nayuta723@gmail.com>

Add custom marker rules and validation logic.

5684496

Signed-off-by: nayuta-ai <nayuta723@gmail.com>

Enhance marker scope validation by introducing allowDangerousTypes fl…

327dc34

…ag and updating type constraints. Refactor schema type definitions and improve error handling for invalid markers. Add new test cases for various marker scenarios. Signed-off-by: nayuta-ai <nayuta723@gmail.com>

refactor(markerscope): fix linters and add test golden files

2396fd4

Signed-off-by: nayuta-ai <nayuta723@gmail.com>

docs(markerscope): document strictTypeConstraint and scope exclusions

35a6b7b

Signed-off-by: nayuta-ai <nayuta723@gmail.com>

refactor(markerscope): convert config to PascalCase values and named …

9618c3c

…object lists Signed-off-by: nayuta-ai <nayuta723@gmail.com>

refactor(markerscope): separate overrideMarkers and customMarkers wit…

6270f45

…h validation Signed-off-by: nayuta-ai <nayuta723@gmail.com>

feat(markerscope): add default config and enhance documentation

7db9920

Signed-off-by: nayuta-ai <nayuta723@gmail.com>

refactor(markerscope): remove support for float/number types

875b36c

Signed-off-by: nayuta-ai <nayuta723@gmail.com>

refactor(markerscope): replace StrictTypeConstraint with NamedTypeCon…

1e10f36

…straint and improve code quality Signed-off-by: nayuta-ai <nayuta723@gmail.com>

refactor(markers): remove Kubebuilder prefix from SSA topology marker…

f8fb88b

… constants Signed-off-by: nayuta-ai <nayuta723@gmail.com>

refactor(markerscope): improve error handling and code organization

9c0a9b6

Signed-off-by: nayuta-ai <nayuta723@gmail.com>

nayuta723 force-pushed the add-markerscope-linter branch from 6aaaa0d to 9c0a9b6 Compare October 30, 2025 08:53

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

refactor(markerscope): simplify getBasicTypeSchema with nolint:exhaus…

f969d45

…tive Signed-off-by: nayuta-ai <nayuta723@gmail.com>

Add markerscope linter #161

Are you sure you want to change the base?

Add markerscope linter #161

Conversation

nayuta723 commented Oct 26, 2025

Uh oh!

k8s-ci-robot commented Oct 26, 2025

Uh oh!

JoelSpeed left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nayuta723 Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nayuta723 Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nayuta723 commented Oct 30, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nayuta723 Oct 30, 2025 •

edited

Loading

nayuta723 Oct 30, 2025 •

edited

Loading