Skip to content

Commit c5fea18

Browse files
committed
feat: add conflictingmarkers linter to detect mutually exclusive markers
1 parent 4fab82d commit c5fea18

File tree

5 files changed

+333
-0
lines changed

5 files changed

+333
-0
lines changed
Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
/*
2+
Copyright 2025 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
package conflictingmarkers
17+
18+
import (
19+
"fmt"
20+
"go/ast"
21+
22+
"golang.org/x/tools/go/analysis"
23+
"k8s.io/apimachinery/pkg/util/sets"
24+
kalerrors "sigs.k8s.io/kube-api-linter/pkg/analysis/errors"
25+
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/extractjsontags"
26+
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/inspector"
27+
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/markers"
28+
"sigs.k8s.io/kube-api-linter/pkg/analysis/utils"
29+
markersconsts "sigs.k8s.io/kube-api-linter/pkg/markers"
30+
)
31+
32+
const name = "conflictingmarkers"
33+
34+
func init() {
35+
// Register all the markers we care about with the marker registry
36+
markers.DefaultRegistry().Register(markersconsts.OptionalMarker)
37+
markers.DefaultRegistry().Register(markersconsts.RequiredMarker)
38+
markers.DefaultRegistry().Register(markersconsts.DefaultMarker)
39+
markers.DefaultRegistry().Register(markersconsts.KubebuilderOptionalMarker)
40+
markers.DefaultRegistry().Register(markersconsts.KubebuilderRequiredMarker)
41+
markers.DefaultRegistry().Register(markersconsts.KubebuilderDefaultMarker)
42+
markers.DefaultRegistry().Register(markersconsts.K8sOptionalMarker)
43+
markers.DefaultRegistry().Register(markersconsts.K8sRequiredMarker)
44+
}
45+
46+
type analyzer struct {
47+
conflictSets []ConflictSet
48+
}
49+
50+
func newAnalyzer(cfg *ConflictingMarkersConfig) *analysis.Analyzer {
51+
if cfg == nil {
52+
cfg = &ConflictingMarkersConfig{}
53+
}
54+
55+
// Register custom markers from configuration
56+
for _, conflictSet := range cfg.CustomConflicts {
57+
for _, markerID := range conflictSet.SetA {
58+
markers.DefaultRegistry().Register(markerID)
59+
}
60+
61+
for _, markerID := range conflictSet.SetB {
62+
markers.DefaultRegistry().Register(markerID)
63+
}
64+
}
65+
66+
a := &analyzer{
67+
conflictSets: append(defaultConflictSets(), cfg.CustomConflicts...),
68+
}
69+
70+
return &analysis.Analyzer{
71+
Name: name,
72+
Doc: "Check that fields do not have conflicting markers from mutually exclusive sets",
73+
Run: a.run,
74+
Requires: []*analysis.Analyzer{inspector.Analyzer},
75+
}
76+
}
77+
78+
func (a *analyzer) run(pass *analysis.Pass) (any, error) {
79+
inspect, ok := pass.ResultOf[inspector.Analyzer].(inspector.Inspector)
80+
if !ok {
81+
return nil, kalerrors.ErrCouldNotGetInspector
82+
}
83+
84+
inspect.InspectFields(func(field *ast.Field, stack []ast.Node, _ extractjsontags.FieldTagInfo, markersAccess markers.Markers) {
85+
checkField(pass, field, markersAccess, a.conflictSets)
86+
})
87+
88+
return nil, nil //nolint:nilnil
89+
}
90+
91+
func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markers.Markers, conflictSets []ConflictSet) {
92+
if field == nil || len(field.Names) == 0 {
93+
return
94+
}
95+
96+
markers := utils.TypeAwareMarkerCollectionForField(pass, markersAccess, field)
97+
98+
for _, conflictSet := range conflictSets {
99+
checkConflict(pass, field, markers, conflictSet)
100+
}
101+
}
102+
103+
func checkConflict(pass *analysis.Pass, field *ast.Field, markers markers.MarkerSet, conflictSet ConflictSet) {
104+
setAMarkers := sets.New[string]()
105+
setBMarkers := sets.New[string]()
106+
107+
// Collect markers from set A
108+
for _, markerID := range conflictSet.SetA {
109+
if markers.Has(markerID) {
110+
setAMarkers.Insert(markerID)
111+
}
112+
}
113+
114+
// Collect markers from set B
115+
for _, markerID := range conflictSet.SetB {
116+
if markers.Has(markerID) {
117+
setBMarkers.Insert(markerID)
118+
}
119+
}
120+
121+
// If both sets have markers, report the conflict
122+
if setAMarkers.Len() > 0 && setBMarkers.Len() > 0 {
123+
reportConflict(pass, field, conflictSet, setAMarkers, setBMarkers)
124+
}
125+
}
126+
127+
func reportConflict(pass *analysis.Pass, field *ast.Field, conflictSet ConflictSet, setAMarkers, setBMarkers sets.Set[string]) {
128+
pass.Report(analysis.Diagnostic{
129+
Pos: field.Pos(),
130+
Message: fmt.Sprintf("field %s has conflicting markers: %v and %v. %s",
131+
field.Names[0].Name,
132+
sets.List(setAMarkers), sets.List(setBMarkers),
133+
conflictSet.Description),
134+
})
135+
}
136+
137+
func defaultConflictSets() []ConflictSet {
138+
return []ConflictSet{
139+
{
140+
Name: "optional_vs_required",
141+
SetA: []string{markersconsts.OptionalMarker, markersconsts.KubebuilderOptionalMarker, markersconsts.K8sOptionalMarker},
142+
SetB: []string{markersconsts.RequiredMarker, markersconsts.KubebuilderRequiredMarker, markersconsts.K8sRequiredMarker},
143+
Description: "A field cannot be both optional and required",
144+
},
145+
{
146+
Name: "default_vs_required",
147+
SetA: []string{markersconsts.DefaultMarker, markersconsts.KubebuilderDefaultMarker},
148+
SetB: []string{markersconsts.RequiredMarker, markersconsts.KubebuilderRequiredMarker, markersconsts.K8sRequiredMarker},
149+
Description: "A field with a default value cannot be required",
150+
},
151+
}
152+
}
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/*
2+
Copyright 2025 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
package conflictingmarkers
17+
18+
// ConflictingMarkersConfig contains the configuration for the conflictingmarkers linter.
19+
type ConflictingMarkersConfig struct {
20+
// CustomConflicts allows users to define their own sets of conflicting markers.
21+
// Each entry defines a conflict between two sets of markers.
22+
CustomConflicts []ConflictSet `json:"customConflicts"`
23+
}
24+
25+
// ConflictSet represents a conflict between two sets of markers.
26+
// Markers within each set are mutually exclusive with markers in the other set.
27+
type ConflictSet struct {
28+
// Name is a human-readable name for this conflict set.
29+
Name string `json:"name"`
30+
// SetA contains the first set of conflicting markers.
31+
SetA []string `json:"setA"`
32+
// SetB contains the second set of conflicting markers.
33+
SetB []string `json:"setB"`
34+
// Description provides a description of why these markers conflict.
35+
Description string `json:"description"`
36+
}
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
/*
2+
Copyright 2025 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
/*
18+
conflictingmarkers is a linter that detects and reports when mutually exclusive markers are used on the same field.
19+
This prevents common configuration errors and unexpected behavior in Kubernetes API types.
20+
21+
The linter includes built-in checks for the following conflicts:
22+
23+
1. Optional vs Required:
24+
- Set A: optional, +kubebuilder:validation:Optional
25+
- Set B: required, +kubebuilder:validation:Required
26+
- A field cannot be both optional and required
27+
28+
2. Default vs Required:
29+
- Set A: default, +kubebuilder:default
30+
- Set B: required, +kubebuilder:validation:Required
31+
- A field with a default value cannot be required
32+
33+
The linter is configurable and allows users to define their own custom sets of conflicting markers.
34+
Each custom conflict set must specify a name, two sets of markers, and a description of why they conflict.
35+
36+
Example configuration:
37+
```yaml
38+
lintersConfig:
39+
40+
conflictingmarkers:
41+
customConflicts:
42+
- name: "my_custom_conflict"
43+
setA: ["custom:marker1", "custom:marker2"]
44+
setB: ["custom:marker3", "custom:marker4"]
45+
description: "These markers conflict because..."
46+
47+
```
48+
49+
The linter does not provide automatic fixes as it cannot determine which conflicting marker should be removed.
50+
*/
51+
package conflictingmarkers
Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
/*
2+
Copyright 2025 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
package conflictingmarkers
17+
18+
import (
19+
"golang.org/x/tools/go/analysis"
20+
"k8s.io/apimachinery/pkg/util/sets"
21+
"k8s.io/apimachinery/pkg/util/validation/field"
22+
"sigs.k8s.io/kube-api-linter/pkg/analysis/initializer"
23+
"sigs.k8s.io/kube-api-linter/pkg/analysis/registry"
24+
)
25+
26+
func init() {
27+
registry.DefaultRegistry().RegisterLinter(Initializer())
28+
}
29+
30+
// Initializer returns the AnalyzerInitializer for this
31+
// Analyzer so that it can be added to the registry.
32+
func Initializer() initializer.AnalyzerInitializer {
33+
return initializer.NewConfigurableInitializer(
34+
name,
35+
initAnalyzer,
36+
true,
37+
validateConfig,
38+
)
39+
}
40+
41+
// initAnalyzer returns the initialized Analyzer.
42+
func initAnalyzer(cfg *ConflictingMarkersConfig) (*analysis.Analyzer, error) {
43+
return newAnalyzer(cfg), nil
44+
}
45+
46+
// validateConfig validates the configuration in the config.ConflictingMarkersConfig struct.
47+
func validateConfig(cfg *ConflictingMarkersConfig, fldPath *field.Path) field.ErrorList {
48+
if cfg == nil {
49+
return field.ErrorList{}
50+
}
51+
52+
fieldErrors := field.ErrorList{}
53+
nameSet := sets.New[string]()
54+
55+
for i, conflictSet := range cfg.CustomConflicts {
56+
if nameSet.Has(conflictSet.Name) {
57+
fieldErrors = append(fieldErrors, field.Invalid(fldPath.Child("customConflicts").Index(i).Child("name"), conflictSet.Name, "repeated value, names must be unique"))
58+
continue
59+
}
60+
61+
fieldErrors = append(fieldErrors, validateConflictSet(conflictSet, fldPath.Child("customConflicts").Index(i))...)
62+
63+
nameSet.Insert(conflictSet.Name)
64+
}
65+
66+
return fieldErrors
67+
}
68+
69+
func validateConflictSet(conflictSet ConflictSet, fldPath *field.Path) field.ErrorList {
70+
fieldErrors := field.ErrorList{}
71+
72+
if conflictSet.Name == "" {
73+
fieldErrors = append(fieldErrors, field.Required(fldPath.Child("name"), "name is required"))
74+
}
75+
76+
if len(conflictSet.SetA) == 0 {
77+
fieldErrors = append(fieldErrors, field.Required(fldPath.Child("setA"), "setA cannot be empty"))
78+
}
79+
80+
if len(conflictSet.SetB) == 0 {
81+
fieldErrors = append(fieldErrors, field.Required(fldPath.Child("setB"), "setB cannot be empty"))
82+
}
83+
84+
// Check for overlapping markers between sets
85+
setA := sets.New(conflictSet.SetA...)
86+
setB := sets.New(conflictSet.SetB...)
87+
88+
if intersection := setA.Intersection(setB); intersection.Len() > 0 {
89+
fieldErrors = append(fieldErrors, field.Invalid(fldPath, conflictSet, "sets cannot contain overlapping markers"))
90+
}
91+
92+
return fieldErrors
93+
}

pkg/registration/doc.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ package registration
2525
import (
2626
_ "sigs.k8s.io/kube-api-linter/pkg/analysis/commentstart"
2727
_ "sigs.k8s.io/kube-api-linter/pkg/analysis/conditions"
28+
_ "sigs.k8s.io/kube-api-linter/pkg/analysis/conflictingmarkers"
2829
_ "sigs.k8s.io/kube-api-linter/pkg/analysis/duplicatemarkers"
2930
_ "sigs.k8s.io/kube-api-linter/pkg/analysis/integers"
3031
_ "sigs.k8s.io/kube-api-linter/pkg/analysis/jsontags"

0 commit comments

Comments
 (0)