Skip to content

Commit 82580f0

Browse files
committed
feat: add conflictingmarkers linter to detect mutually exclusive markers
1 parent cf6e504 commit 82580f0

File tree

5 files changed

+321
-0
lines changed

5 files changed

+321
-0
lines changed
Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
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+
)
30+
31+
const name = "conflictingmarkers"
32+
33+
type analyzer struct {
34+
conflictSets []ConflictSet
35+
}
36+
37+
func newAnalyzer(cfg *ConflictingMarkersConfig) *analysis.Analyzer {
38+
if cfg == nil {
39+
cfg = &ConflictingMarkersConfig{}
40+
}
41+
42+
// Register markers from configuration
43+
for _, conflictSet := range cfg.Conflicts {
44+
for _, markerID := range conflictSet.SetA {
45+
markers.DefaultRegistry().Register(markerID)
46+
}
47+
48+
for _, markerID := range conflictSet.SetB {
49+
markers.DefaultRegistry().Register(markerID)
50+
}
51+
}
52+
53+
a := &analyzer{
54+
conflictSets: cfg.Conflicts,
55+
}
56+
57+
return &analysis.Analyzer{
58+
Name: name,
59+
Doc: "Check that fields do not have conflicting markers from mutually exclusive sets",
60+
Run: a.run,
61+
Requires: []*analysis.Analyzer{inspector.Analyzer},
62+
}
63+
}
64+
65+
func (a *analyzer) run(pass *analysis.Pass) (any, error) {
66+
inspect, ok := pass.ResultOf[inspector.Analyzer].(inspector.Inspector)
67+
if !ok {
68+
return nil, kalerrors.ErrCouldNotGetInspector
69+
}
70+
71+
inspect.InspectFields(func(field *ast.Field, stack []ast.Node, _ extractjsontags.FieldTagInfo, markersAccess markers.Markers) {
72+
checkField(pass, field, markersAccess, a.conflictSets)
73+
})
74+
75+
return nil, nil //nolint:nilnil
76+
}
77+
78+
func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markers.Markers, conflictSets []ConflictSet) {
79+
if field == nil || len(field.Names) == 0 {
80+
return
81+
}
82+
83+
markers := utils.TypeAwareMarkerCollectionForField(pass, markersAccess, field)
84+
85+
for _, conflictSet := range conflictSets {
86+
checkConflict(pass, field, markers, conflictSet)
87+
}
88+
}
89+
90+
func checkConflict(pass *analysis.Pass, field *ast.Field, markers markers.MarkerSet, conflictSet ConflictSet) {
91+
setAMarkers := sets.New[string]()
92+
setBMarkers := sets.New[string]()
93+
94+
// Collect markers from set A
95+
for _, markerID := range conflictSet.SetA {
96+
if markers.Has(markerID) {
97+
setAMarkers.Insert(markerID)
98+
}
99+
}
100+
101+
// Collect markers from set B
102+
for _, markerID := range conflictSet.SetB {
103+
if markers.Has(markerID) {
104+
setBMarkers.Insert(markerID)
105+
}
106+
}
107+
108+
// If both sets have markers, report the conflict
109+
if setAMarkers.Len() > 0 && setBMarkers.Len() > 0 {
110+
reportConflict(pass, field, conflictSet, setAMarkers, setBMarkers)
111+
}
112+
}
113+
114+
func reportConflict(pass *analysis.Pass, field *ast.Field, conflictSet ConflictSet, setAMarkers, setBMarkers sets.Set[string]) {
115+
pass.Report(analysis.Diagnostic{
116+
Pos: field.Pos(),
117+
Message: fmt.Sprintf("field %s has conflicting markers: %s: %v and %v. %s",
118+
field.Names[0].Name,
119+
conflictSet.Name,
120+
sets.List(setAMarkers), sets.List(setBMarkers),
121+
conflictSet.Description),
122+
})
123+
}
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
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+
// Conflicts allows users to define sets of conflicting markers.
21+
// Each entry defines a conflict between two sets of markers.
22+
Conflicts []ConflictSet `json:"conflicts"`
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+
// This name will appear in diagnostic messages to identify the type of conflict,
30+
Name string `json:"name"`
31+
// SetA contains the first set of markers that conflict with markers in SetB.
32+
// These markers are mutually exclusive with markers in setB.
33+
// The linter will emit a diagnostic when a field has markers from both setA and setB.
34+
SetA []string `json:"setA"`
35+
// SetB contains the second set of markers that conflict with markers in SetA.
36+
// These markers are mutually exclusive with markers in setA.
37+
// The linter will emit a diagnostic when a field has markers from both setA and setB.
38+
SetB []string `json:"setB"`
39+
// Description provides a description of why these markers conflict.
40+
// The linter will include this description in the diagnostic message when a conflict is detected.
41+
Description string `json:"description"`
42+
}
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
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 reports issues when markers from both sets of a conflict pair are present on the same field.
22+
It does NOT report issues when multiple markers from the same set are present - only when markers from
23+
different sets within the same conflict definition are found together.
24+
25+
The linter is fully configurable and requires users to define all conflict sets they want to check.
26+
There are no built-in conflict sets - all conflicts must be explicitly configured.
27+
28+
Each conflict set must specify:
29+
- A unique name for the conflict
30+
- Two sets of markers that are mutually exclusive
31+
- A description explaining why the markers conflict
32+
33+
Example configuration:
34+
```yaml
35+
lintersConfig:
36+
37+
conflictingmarkers:
38+
conflicts:
39+
- name: "optional_vs_required"
40+
setA: ["optional", "+kubebuilder:validation:Optional", "+k8s:validation:optional"]
41+
setB: ["required", "+kubebuilder:validation:Required", "+k8s:validation:required"]
42+
description: "A field cannot be both optional and required"
43+
- name: "default_vs_required"
44+
setA: ["default", "+kubebuilder:default"]
45+
setB: ["required", "+kubebuilder:validation:Required", "+k8s:validation:required"]
46+
description: "A field with a default value cannot be required"
47+
- name: "my_custom_conflict"
48+
setA: ["custom:marker1", "custom:marker2"]
49+
setB: ["custom:marker3", "custom:marker4"]
50+
description: "These markers conflict because..."
51+
52+
```
53+
54+
Note: This linter is not enabled by default and must be explicitly enabled in the configuration.
55+
56+
The linter does not provide automatic fixes as it cannot determine which conflicting marker should be removed.
57+
*/
58+
package conflictingmarkers
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
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+
false,
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.Conflicts {
56+
if nameSet.Has(conflictSet.Name) {
57+
fieldErrors = append(fieldErrors, field.Invalid(fldPath.Child("conflicts").Index(i).Child("name"), conflictSet.Name, "repeated value, names must be unique"))
58+
continue
59+
}
60+
61+
fieldErrors = append(fieldErrors, validateConflictSet(conflictSet, fldPath.Child("conflicts").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 conflictSet.Description == "" {
77+
fieldErrors = append(fieldErrors, field.Required(fldPath.Child("description"), "description is required"))
78+
}
79+
80+
if len(conflictSet.SetA) == 0 {
81+
fieldErrors = append(fieldErrors, field.Required(fldPath.Child("setA"), "setA cannot be empty"))
82+
}
83+
84+
if len(conflictSet.SetB) == 0 {
85+
fieldErrors = append(fieldErrors, field.Required(fldPath.Child("setB"), "setB cannot be empty"))
86+
}
87+
88+
// Check for overlapping markers between sets
89+
setA := sets.New(conflictSet.SetA...)
90+
setB := sets.New(conflictSet.SetB...)
91+
92+
if intersection := setA.Intersection(setB); intersection.Len() > 0 {
93+
fieldErrors = append(fieldErrors, field.Invalid(fldPath, conflictSet, "sets cannot contain overlapping markers"))
94+
}
95+
96+
return fieldErrors
97+
}

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)