Skip to content

Commit 6270f45

Browse files
committed
refactor(markerscope): separate overrideMarkers and customMarkers with validation
Signed-off-by: nayuta-ai <nayuta723@gmail.com>
1 parent 9618c3c commit 6270f45

File tree

8 files changed

+314
-107
lines changed

8 files changed

+314
-107
lines changed

docs/linters.md

Lines changed: 33 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -433,33 +433,52 @@ The linter includes built-in rules for all standard kubebuilder markers and k8s
433433

434434
### Configuration
435435

436-
You can customize marker rules or add support for custom markers:
436+
You can customize marker rules or add support for custom markers.
437+
438+
**Scope values:**
439+
- `Field`: Marker can only be applied to struct fields
440+
- `Type`: Marker can only be applied to type definitions
441+
- `Any`: Marker can be applied to either fields or type definitions
442+
443+
**Type constraints:**
444+
445+
The `typeConstraint` field allows you to restrict which Go types a marker can be applied to. This ensures that markers are only used with compatible data types (e.g., numeric markers like `Minimum` are only applied to integer/number types).
446+
447+
**Type constraint fields:**
448+
- `allowedSchemaTypes`: List of allowed OpenAPI schema types (`integer`, `number`, `string`, `boolean`, `array`, `object`)
449+
- `elementConstraint`: Nested constraint for array element types (only valid when `allowedSchemaTypes` includes `array`)
450+
- `strictTypeConstraint`: When `true`, markers with `AnyScope` and type constraints applied to fields using named types must be declared on the type definition instead of the field. Defaults to `false`.
451+
452+
**Configuration example:**
437453

438454
```yaml
439455
lintersConfig:
440456
markerscope:
441457
policy: Warn | SuggestFix # The policy for marker scope violations. Defaults to `Warn`.
442458
allowDangerousTypes: false # Allow dangerous number types (float32, float64). Defaults to `false`.
443-
markerRules:
444-
# Override default rule for a built-in marker
445-
- name: "optional"
459+
460+
# Override default rules for built-in markers
461+
overrideMarkers:
462+
- identifier: "optional"
446463
scope: Field # or: Type, Any
447464

448-
# Add a custom marker with scope constraint only
449-
- name: "mycompany:validation:CustomMarker"
465+
# Add rules for custom markers
466+
customMarkers:
467+
# Custom marker with scope constraint only
468+
- identifier: "mycompany:validation:CustomMarker"
450469
scope: Any
451470

452-
# Add a custom marker with scope and type constraints
453-
- name: "mycompany:validation:NumericLimit"
471+
# Custom marker with scope and type constraints
472+
- identifier: "mycompany:validation:NumericLimit"
454473
scope: Any
455474
strictTypeConstraint: true # Require declaration on type definition for named types
456475
typeConstraint:
457476
allowedSchemaTypes:
458477
- integer
459478
- number
460479

461-
# Add a custom array items marker with element type constraint
462-
- name: "mycompany:validation:items:StringFormat"
480+
# Custom array items marker with element type constraint
481+
- identifier: "mycompany:validation:items:StringFormat"
463482
scope: Any
464483
typeConstraint:
465484
allowedSchemaTypes:
@@ -469,18 +488,10 @@ lintersConfig:
469488
- string
470489
```
471490
472-
**Scope values:**
473-
- `Field`: FieldScope - marker can only be on fields
474-
- `Type`: TypeScope - marker can only be on types
475-
- `Any`: AnyScope - marker can be on fields or types
476-
477-
**Type constraint fields:**
478-
- `allowedSchemaTypes`: List of allowed OpenAPI schema types (`integer`, `number`, `string`, `boolean`, `array`, `object`)
479-
- `elementConstraint`: Nested constraint for array element types (only valid when `allowedSchemaTypes` includes `array`)
480-
- `strictTypeConstraint`: When `true`, markers with `AnyScope` and type constraints applied to fields using named types must be declared on the type definition instead of the field. Defaults to `false`.
481-
482-
If a marker is not in `markerRules` and not in the default rules, no validation is performed for that marker.
483-
If a marker is in both `markerRules` and the default rules, your configuration takes precedence.
491+
**Configuration notes:**
492+
- Use `overrideMarkers` to customize the behavior of built-in kubebuilder/controller-runtime markers
493+
- Use `customMarkers` to add validation for your own custom markers
494+
- If a marker is not in either list and not in the default rules, no validation is performed for that marker
484495

485496
### Fixes
486497

pkg/analysis/markerscope/analyzer.go

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,22 @@ func newAnalyzer(cfg *MarkerScopeConfig) *analysis.Analyzer {
6060
cfg = &MarkerScopeConfig{}
6161
}
6262

63-
// Convert list of marker rules to map
64-
customRules := markerRulesListToMap(cfg.MarkerRules)
63+
// Convert override and custom marker lists to maps
64+
overrideRules := markerRulesListToMap(cfg.OverrideMarkers)
65+
customRules := markerRulesListToMap(cfg.CustomMarkers)
66+
67+
// Merge rules:
68+
// 1. Start with default built-in marker rules
69+
// 2. Apply overrides (replaces default rules for built-in markers)
70+
// 3. Add custom markers (new markers not in defaults)
71+
// Note: Validation ensures overrideMarkers only contains built-in markers
72+
// and customMarkers only contains non-built-in markers, so no conflicts.
73+
rules := DefaultMarkerRules()
74+
maps.Copy(rules, overrideRules) // Override built-in markers
75+
maps.Copy(rules, customRules) // Add custom markers
6576

6677
a := &analyzer{
67-
markerRules: mergeMarkerRules(DefaultMarkerRules(), customRules),
78+
markerRules: rules,
6879
policy: cfg.Policy,
6980
allowDangerousTypes: cfg.AllowDangerousTypes,
7081
}
@@ -90,31 +101,17 @@ func newAnalyzer(cfg *MarkerScopeConfig) *analysis.Analyzer {
90101
}
91102
}
92103

93-
// markerRulesListToMap converts a list of marker rules to a map keyed by marker name.
104+
// markerRulesListToMap converts a list of marker rules to a map keyed by marker identifier.
94105
func markerRulesListToMap(rules []MarkerScopeRule) map[string]MarkerScopeRule {
95106
result := make(map[string]MarkerScopeRule, len(rules))
96107
for _, rule := range rules {
97-
if rule.Name != "" {
98-
result[rule.Name] = rule
108+
if rule.Identifier != "" {
109+
result[rule.Identifier] = rule
99110
}
100111
}
101112
return result
102113
}
103114

104-
// mergeMarkerRules merges custom marker rules with default marker rules.
105-
// Custom rules take precedence over default rules for the same marker.
106-
func mergeMarkerRules(defaults, custom map[string]MarkerScopeRule) map[string]MarkerScopeRule {
107-
merged := make(map[string]MarkerScopeRule, len(defaults)+len(custom))
108-
109-
// Copy all default rules
110-
maps.Copy(merged, defaults)
111-
112-
// Override with custom rules
113-
maps.Copy(merged, custom)
114-
115-
return merged
116-
}
117-
118115
func (a *analyzer) run(pass *analysis.Pass) (any, error) {
119116
inspect, ok := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
120117
if !ok {

pkg/analysis/markerscope/analyzer_test.go

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -39,41 +39,53 @@ func TestAnalyzerSuggestFixes(t *testing.T) {
3939
analysistest.RunWithSuggestedFixes(t, testdata, analyzer, "a")
4040
}
4141

42-
func TestAnalyzerWithCustomMarkers(t *testing.T) {
42+
func TestAnalyzerWithCustomAndOverrideMarkers(t *testing.T) {
4343
testdata := analysistest.TestData()
4444
cfg := &MarkerScopeConfig{
4545
Policy: MarkerScopePolicyWarn,
46-
MarkerRules: []MarkerScopeRule{
46+
OverrideMarkers: []MarkerScopeRule{
47+
// Override built-in "optional" to allow on types (default is FieldScope only)
48+
{
49+
Identifier: "optional",
50+
Scope: AnyScope,
51+
},
52+
// Override built-in "required" to allow on types (default is FieldScope only)
53+
{
54+
Identifier: "required",
55+
Scope: AnyScope,
56+
},
57+
},
58+
CustomMarkers: []MarkerScopeRule{
4759
// Custom field-only marker
4860
{
49-
Name: "custom:field-only",
50-
Scope: FieldScope,
61+
Identifier: "custom:field-only",
62+
Scope: FieldScope,
5163
},
5264
// Custom type-only marker
5365
{
54-
Name: "custom:type-only",
55-
Scope: TypeScope,
66+
Identifier: "custom:type-only",
67+
Scope: TypeScope,
5668
},
5769
// Custom marker with string type constraint
5870
{
59-
Name: "custom:string-only",
60-
Scope: FieldScope,
71+
Identifier: "custom:string-only",
72+
Scope: FieldScope,
6173
TypeConstraint: &TypeConstraint{
6274
AllowedSchemaTypes: []SchemaType{SchemaTypeString},
6375
},
6476
},
6577
// Custom marker with integer type constraint
6678
{
67-
Name: "custom:integer-only",
68-
Scope: FieldScope,
79+
Identifier: "custom:integer-only",
80+
Scope: FieldScope,
6981
TypeConstraint: &TypeConstraint{
7082
AllowedSchemaTypes: []SchemaType{SchemaTypeInteger},
7183
},
7284
},
7385
// Custom marker with array of strings constraint
7486
{
75-
Name: "custom:string-array",
76-
Scope: FieldScope,
87+
Identifier: "custom:string-array",
88+
Scope: FieldScope,
7789
TypeConstraint: &TypeConstraint{
7890
AllowedSchemaTypes: []SchemaType{SchemaTypeArray},
7991
ElementConstraint: &TypeConstraint{

pkg/analysis/markerscope/config.go

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,8 @@ type TypeConstraint struct {
5757

5858
// MarkerScopeRule defines comprehensive scope validation rules for a marker.
5959
type MarkerScopeRule struct {
60-
// Name is the marker identifier (e.g., "optional", "kubebuilder:validation:Minimum").
61-
// This field is only used when MarkerScopeRule is part of a list configuration.
62-
Name string `json:"name,omitempty"`
60+
// Identifier is the marker identifier (e.g., "optional", "kubebuilder:validation:Minimum").
61+
Identifier string `json:"identifier,omitempty"`
6362

6463
// Scope specifies where the marker can be placed (field vs type).
6564
Scope ScopeConstraint
@@ -89,21 +88,25 @@ const (
8988

9089
// MarkerScopeConfig contains configuration for marker scope validation.
9190
type MarkerScopeConfig struct {
92-
// MarkerRules is a list of marker rules with scope and type constraints.
93-
// This list can be used to:
94-
// - Override default rules for built-in markers (from DefaultMarkerRules)
95-
// - Add rules for custom markers not included in DefaultMarkerRules
91+
// OverrideMarkers is a list of marker rules that override default rules for built-in markers.
92+
// Use this to customize the behavior of standard kubebuilder/controller-runtime markers.
9693
//
97-
// If a marker is not in this list AND not in DefaultMarkerRules(), no scope validation is performed.
98-
// If a marker is in both this list and DefaultMarkerRules(), this list takes precedence.
94+
// Example: Override the built-in "optional" marker
95+
// overrideMarkers:
96+
// - identifier: "optional"
97+
// scope: Field
98+
OverrideMarkers []MarkerScopeRule `json:"overrideMarkers,omitempty"`
99+
100+
// CustomMarkers is a list of marker rules for custom markers not included in the default rules.
101+
// Use this to add validation for your own custom markers.
99102
//
100-
// Example: Adding a custom marker
101-
// markerRules:
102-
// - name: "mycompany:validation:CustomMarker"
103-
// scope: any
103+
// Example: Add a custom marker
104+
// customMarkers:
105+
// - identifier: "mycompany:validation:CustomMarker"
106+
// scope: Any
104107
// typeConstraint:
105108
// allowedSchemaTypes: ["string"]
106-
MarkerRules []MarkerScopeRule `json:"markerRules,omitempty"`
109+
CustomMarkers []MarkerScopeRule `json:"customMarkers,omitempty"`
107110

108111
// AllowDangerousTypes specifies if dangerous types are allowed.
109112
// If true, dangerous types are allowed.

pkg/analysis/markerscope/initializer.go

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,13 +57,45 @@ func validateConfig(cfg *MarkerScopeConfig, fldPath *field.Path) field.ErrorList
5757
fmt.Sprintf("invalid policy, must be one of: %q, %q", MarkerScopePolicyWarn, MarkerScopePolicySuggestFix)))
5858
}
5959

60-
// Validate marker rules
61-
for i, rule := range cfg.MarkerRules {
62-
markerRulePath := fldPath.Child("markerRules").Index(i)
60+
// Get default marker rules for validation
61+
defaultRules := DefaultMarkerRules()
6362

64-
// Validate that name is not empty
65-
if rule.Name == "" {
66-
fieldErrors = append(fieldErrors, field.Required(markerRulePath.Child("name"), "marker name is required"))
63+
// Validate override marker rules
64+
for i, rule := range cfg.OverrideMarkers {
65+
markerRulePath := fldPath.Child("overrideMarkers").Index(i)
66+
67+
// Validate that identifier is not empty
68+
if rule.Identifier == "" {
69+
fieldErrors = append(fieldErrors, field.Required(markerRulePath.Child("identifier"), "marker identifier is required"))
70+
continue
71+
}
72+
73+
// Validate that override marker exists in default rules
74+
if _, exists := defaultRules[rule.Identifier]; !exists {
75+
fieldErrors = append(fieldErrors, field.Invalid(markerRulePath.Child("identifier"), rule.Identifier,
76+
"override marker must be a built-in marker; use customMarkers for custom markers"))
77+
continue
78+
}
79+
80+
if err := validateMarkerRule(rule); err != nil {
81+
fieldErrors = append(fieldErrors, field.Invalid(markerRulePath, rule, err.Error()))
82+
}
83+
}
84+
85+
// Validate custom marker rules
86+
for i, rule := range cfg.CustomMarkers {
87+
markerRulePath := fldPath.Child("customMarkers").Index(i)
88+
89+
// Validate that identifier is not empty
90+
if rule.Identifier == "" {
91+
fieldErrors = append(fieldErrors, field.Required(markerRulePath.Child("identifier"), "marker identifier is required"))
92+
continue
93+
}
94+
95+
// Validate that custom marker does not exist in default rules
96+
if _, exists := defaultRules[rule.Identifier]; exists {
97+
fieldErrors = append(fieldErrors, field.Invalid(markerRulePath.Child("identifier"), rule.Identifier,
98+
"custom marker cannot be a built-in marker; use overrideMarkers to override built-in markers"))
6799
continue
68100
}
69101

0 commit comments

Comments
 (0)