Skip to content

Commit 47bfeef

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #126 from yongruilin/conflictmarkers
feat: add Conflictingmarkers
2 parents ab7a290 + 90a97b3 commit 47bfeef

File tree

10 files changed

+708
-0
lines changed

10 files changed

+708
-0
lines changed

‎docs/linters.md

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
- [Conditions](#conditions) - Checks that `Conditions` fields are correctly formatted
44
- [CommentStart](#commentstart) - Ensures comments start with the serialized form of the type
5+
- [ConflictingMarkers](#conflictingmarkers) - Detects mutually exclusive markers on the same field
56
- [DuplicateMarkers](#duplicatemarkers) - Checks for exact duplicates of markers
67
- [Integers](#integers) - Validates usage of supported integer types
78
- [JSONTags](#jsontags) - Ensures proper JSON tag formatting
@@ -74,6 +75,49 @@ The `commentstart` linter can automatically fix comments that do not start with
7475

7576
When the `json` tag is present, and matches the first word of the field comment in all but casing, the linter will suggest that the comment be updated to match the `json` tag.
7677

78+
## ConflictingMarkers
79+
80+
The `conflictingmarkers` linter detects and reports when mutually exclusive markers are used on the same field.
81+
This prevents common configuration errors and unexpected behavior in Kubernetes API types.
82+
83+
The linter reports issues when markers from two or more sets of a conflict definition are present on the same field.
84+
It does NOT report issues when multiple markers from the same set are present - only when markers from
85+
different sets within the same conflict definition are found together.
86+
87+
The linter is configurable and allows users to define sets of conflicting markers.
88+
Each conflict set must specify:
89+
- A unique name for the conflict
90+
- Multiple sets of markers that are mutually exclusive with each other (at least 2 sets)
91+
- A description explaining why the markers conflict
92+
93+
### Configuration
94+
95+
```yaml
96+
lintersConfig:
97+
conflictingmarkers:
98+
conflicts:
99+
- name: "optional_vs_required"
100+
sets:
101+
- ["optional", "+kubebuilder:validation:Optional", "+k8s:validation:optional"]
102+
- ["required", "+kubebuilder:validation:Required", "+k8s:validation:required"]
103+
description: "A field cannot be both optional and required"
104+
- name: "default_vs_required"
105+
sets:
106+
- ["default", "+kubebuilder:default"]
107+
- ["required", "+kubebuilder:validation:Required", "+k8s:validation:required"]
108+
description: "A field with a default value cannot be required"
109+
- name: "three_way_conflict"
110+
sets:
111+
- ["marker5", "marker6"]
112+
- ["marker7", "marker8"]
113+
- ["marker9", "marker10"]
114+
description: "Three-way conflict between marker sets"
115+
```
116+
117+
**Note**: This linter is not enabled by default and must be explicitly enabled in the configuration.
118+
119+
The linter does not provide automatic fixes as it cannot determine which conflicting marker should be removed.
120+
77121
## DuplicateMarkers
78122

79123
The duplicatemarkers linter checks for exact duplicates of markers for types and fields.

‎pkg/analysis/conflictingmarkers/analyzer.go

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
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+
"strings"
22+
23+
"golang.org/x/tools/go/analysis"
24+
"k8s.io/apimachinery/pkg/util/sets"
25+
kalerrors "sigs.k8s.io/kube-api-linter/pkg/analysis/errors"
26+
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/extractjsontags"
27+
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/inspector"
28+
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/markers"
29+
"sigs.k8s.io/kube-api-linter/pkg/analysis/utils"
30+
)
31+
32+
const name = "conflictingmarkers"
33+
34+
type analyzer struct {
35+
conflictSets []ConflictSet
36+
}
37+
38+
func newAnalyzer(cfg *ConflictingMarkersConfig) *analysis.Analyzer {
39+
if cfg == nil {
40+
cfg = &ConflictingMarkersConfig{}
41+
}
42+
43+
// Register markers from configuration
44+
for _, conflictSet := range cfg.Conflicts {
45+
for _, set := range conflictSet.Sets {
46+
for _, markerID := range set {
47+
markers.DefaultRegistry().Register(markerID)
48+
}
49+
}
50+
}
51+
52+
a := &analyzer{
53+
conflictSets: cfg.Conflicts,
54+
}
55+
56+
return &analysis.Analyzer{
57+
Name: name,
58+
Doc: "Check that fields do not have conflicting markers from mutually exclusive sets",
59+
Run: a.run,
60+
Requires: []*analysis.Analyzer{inspector.Analyzer},
61+
}
62+
}
63+
64+
func (a *analyzer) run(pass *analysis.Pass) (any, error) {
65+
inspect, ok := pass.ResultOf[inspector.Analyzer].(inspector.Inspector)
66+
if !ok {
67+
return nil, kalerrors.ErrCouldNotGetInspector
68+
}
69+
70+
inspect.InspectFields(func(field *ast.Field, stack []ast.Node, _ extractjsontags.FieldTagInfo, markersAccess markers.Markers) {
71+
checkField(pass, field, markersAccess, a.conflictSets)
72+
})
73+
74+
return nil, nil //nolint:nilnil
75+
}
76+
77+
func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markers.Markers, conflictSets []ConflictSet) {
78+
if field == nil || len(field.Names) == 0 {
79+
return
80+
}
81+
82+
markers := utils.TypeAwareMarkerCollectionForField(pass, markersAccess, field)
83+
84+
for _, conflictSet := range conflictSets {
85+
checkConflict(pass, field, markers, conflictSet)
86+
}
87+
}
88+
89+
func checkConflict(pass *analysis.Pass, field *ast.Field, markers markers.MarkerSet, conflictSet ConflictSet) {
90+
// Track which sets have markers present
91+
conflictingMarkers := make([]sets.Set[string], 0)
92+
93+
for _, set := range conflictSet.Sets {
94+
foundMarkers := sets.New[string]()
95+
96+
for _, markerID := range set {
97+
if markers.Has(markerID) {
98+
foundMarkers.Insert(markerID)
99+
}
100+
}
101+
// Only add the set if it has at least one marker
102+
if foundMarkers.Len() > 0 {
103+
conflictingMarkers = append(conflictingMarkers, foundMarkers)
104+
}
105+
}
106+
107+
// If two or more sets have markers, report the conflict
108+
if len(conflictingMarkers) >= 2 {
109+
reportConflict(pass, field, conflictSet, conflictingMarkers)
110+
}
111+
}
112+
113+
func reportConflict(pass *analysis.Pass, field *ast.Field, conflictSet ConflictSet, conflictingMarkers []sets.Set[string]) {
114+
// Build a descriptive message showing which sets conflict
115+
setDescriptions := make([]string, 0, len(conflictingMarkers))
116+
117+
for _, set := range conflictingMarkers {
118+
markersList := sets.List(set)
119+
setDescriptions = append(setDescriptions, fmt.Sprintf("%v", markersList))
120+
}
121+
122+
message := fmt.Sprintf("field %s has conflicting markers: %s: {%s}. %s",
123+
field.Names[0].Name,
124+
conflictSet.Name,
125+
strings.Join(setDescriptions, ", "),
126+
conflictSet.Description)
127+
128+
pass.Report(analysis.Diagnostic{
129+
Pos: field.Pos(),
130+
Message: message,
131+
})
132+
}

‎pkg/analysis/conflictingmarkers/analyzer_test.go

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+
package conflictingmarkers_test
17+
18+
import (
19+
"testing"
20+
21+
"golang.org/x/tools/go/analysis/analysistest"
22+
"sigs.k8s.io/kube-api-linter/pkg/analysis/conflictingmarkers"
23+
)
24+
25+
func TestConflictingMarkersAnalyzer(t *testing.T) {
26+
testdata := analysistest.TestData()
27+
28+
config := &conflictingmarkers.ConflictingMarkersConfig{
29+
Conflicts: []conflictingmarkers.ConflictSet{
30+
{
31+
Name: "test_conflict",
32+
Sets: [][]string{{"marker1", "marker2"}, {"marker3", "marker4"}},
33+
Description: "Test markers conflict with each other",
34+
},
35+
{
36+
Name: "three_way_conflict",
37+
Sets: [][]string{{"marker5", "marker6"}, {"marker7", "marker8"}, {"marker9", "marker10"}},
38+
Description: "Three-way conflict between marker sets",
39+
},
40+
},
41+
}
42+
43+
initializer := conflictingmarkers.Initializer()
44+
45+
analyzer, err := initializer.Init(config)
46+
if err != nil {
47+
t.Fatal(err)
48+
}
49+
50+
analysistest.Run(t, testdata, analyzer, "a")
51+
}

‎pkg/analysis/conflictingmarkers/config.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
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 multiple sets of markers.
22+
Conflicts []ConflictSet `json:"conflicts"`
23+
}
24+
25+
// ConflictSet represents a conflict between multiple sets of markers.
26+
// Markers within each set are mutually exclusive with markers in all other sets.
27+
// The linter will emit a diagnostic when a field has markers from two or more sets.
28+
type ConflictSet struct {
29+
// Name is a human-readable name for this conflict set.
30+
// This name will appear in diagnostic messages to identify the type of conflict.
31+
Name string `json:"name"`
32+
// Sets contains the sets of markers that are mutually exclusive with each other.
33+
// Each set is a slice of marker identifiers.
34+
// The linter will emit a diagnostic when a field has markers from two or more sets.
35+
Sets [][]string `json:"sets"`
36+
// Description provides a description of why these markers conflict.
37+
// The linter will include this description in the diagnostic message when a conflict is detected.
38+
Description string `json:"description"`
39+
}

‎pkg/analysis/conflictingmarkers/conflictingmarkers_suite_test.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
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+
package conflictingmarkers_test
18+
19+
import (
20+
"testing"
21+
22+
. "github.com/onsi/ginkgo/v2"
23+
. "github.com/onsi/gomega"
24+
)
25+
26+
func TestConflictingMarkers(t *testing.T) {
27+
RegisterFailHandler(Fail)
28+
RunSpecs(t, "conflictingmarkers")
29+
}

‎pkg/analysis/conflictingmarkers/doc.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
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 two or more sets of a conflict definition 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+
- Multiple sets of markers that are mutually exclusive with each other (at least 2 sets)
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+
sets:
41+
- ["optional", "+kubebuilder:validation:Optional", "+k8s:validation:optional"]
42+
- ["required", "+kubebuilder:validation:Required", "+k8s:validation:required"]
43+
description: "A field cannot be both optional and required"
44+
- name: "my_custom_conflict"
45+
sets:
46+
- ["custom:marker1", "custom:marker2"]
47+
- ["custom:marker3", "custom:marker4"]
48+
- ["custom:marker5", "custom:marker6"]
49+
description: "These markers define different storage backends that cannot be used simultaneously"
50+
51+
```
52+
53+
Configuration options:
54+
- `conflicts`: Required list of conflict set definitions.
55+
56+
Note: This linter is not enabled by default and must be explicitly enabled in the configuration.
57+
58+
The linter does not provide automatic fixes as it cannot determine which conflicting marker should be removed.
59+
*/
60+
package conflictingmarkers

0 commit comments

Comments
 (0)