Skip to content

feat: add Conflictingmarkers #126

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 29, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions docs/linters.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

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

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.

## ConflictingMarkers

The `conflictingmarkers` linter detects and reports when mutually exclusive markers are used on the same field.
This prevents common configuration errors and unexpected behavior in Kubernetes API types.

The linter reports issues when markers from two or more sets of a conflict definition are present on the same field.
It does NOT report issues when multiple markers from the same set are present - only when markers from
different sets within the same conflict definition are found together.

The linter is configurable and allows users to define sets of conflicting markers.
Each conflict set must specify:
- A unique name for the conflict
- Multiple sets of markers that are mutually exclusive with each other (at least 2 sets)
- A description explaining why the markers conflict

### Configuration

```yaml
lintersConfig:
conflictingmarkers:
conflicts:
- name: "optional_vs_required"
sets:
- ["optional", "+kubebuilder:validation:Optional", "+k8s:validation:optional"]
- ["required", "+kubebuilder:validation:Required", "+k8s:validation:required"]
description: "A field cannot be both optional and required"
- name: "default_vs_required"
sets:
- ["default", "+kubebuilder:default"]
- ["required", "+kubebuilder:validation:Required", "+k8s:validation:required"]
description: "A field with a default value cannot be required"
- name: "three_way_conflict"
sets:
- ["marker5", "marker6"]
- ["marker7", "marker8"]
- ["marker9", "marker10"]
description: "Three-way conflict between marker sets"
```

**Note**: This linter is not enabled by default and must be explicitly enabled in the configuration.

The linter does not provide automatic fixes as it cannot determine which conflicting marker should be removed.

## DuplicateMarkers

The duplicatemarkers linter checks for exact duplicates of markers for types and fields.
Expand Down
132 changes: 132 additions & 0 deletions pkg/analysis/conflictingmarkers/analyzer.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
/*
Copyright 2025 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package conflictingmarkers

import (
"fmt"
"go/ast"
"strings"

"golang.org/x/tools/go/analysis"
"k8s.io/apimachinery/pkg/util/sets"
kalerrors "sigs.k8s.io/kube-api-linter/pkg/analysis/errors"
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/extractjsontags"
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/inspector"
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/markers"
"sigs.k8s.io/kube-api-linter/pkg/analysis/utils"
)

const name = "conflictingmarkers"

type analyzer struct {
conflictSets []ConflictSet
}

func newAnalyzer(cfg *ConflictingMarkersConfig) *analysis.Analyzer {
if cfg == nil {
cfg = &ConflictingMarkersConfig{}
}

// Register markers from configuration
for _, conflictSet := range cfg.Conflicts {
for _, set := range conflictSet.Sets {
for _, markerID := range set {
markers.DefaultRegistry().Register(markerID)
}
}
}

a := &analyzer{
conflictSets: cfg.Conflicts,
}

return &analysis.Analyzer{
Name: name,
Doc: "Check that fields do not have conflicting markers from mutually exclusive sets",
Run: a.run,
Requires: []*analysis.Analyzer{inspector.Analyzer},
}
}

func (a *analyzer) run(pass *analysis.Pass) (any, error) {
inspect, ok := pass.ResultOf[inspector.Analyzer].(inspector.Inspector)
if !ok {
return nil, kalerrors.ErrCouldNotGetInspector
}

inspect.InspectFields(func(field *ast.Field, stack []ast.Node, _ extractjsontags.FieldTagInfo, markersAccess markers.Markers) {
checkField(pass, field, markersAccess, a.conflictSets)
})

return nil, nil //nolint:nilnil
}

func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markers.Markers, conflictSets []ConflictSet) {
if field == nil || len(field.Names) == 0 {
return
}

markers := utils.TypeAwareMarkerCollectionForField(pass, markersAccess, field)

for _, conflictSet := range conflictSets {
checkConflict(pass, field, markers, conflictSet)
}
}

func checkConflict(pass *analysis.Pass, field *ast.Field, markers markers.MarkerSet, conflictSet ConflictSet) {
// Track which sets have markers present
conflictingMarkers := make([]sets.Set[string], 0)

for _, set := range conflictSet.Sets {
foundMarkers := sets.New[string]()

for _, markerID := range set {
if markers.Has(markerID) {
foundMarkers.Insert(markerID)
}
}
// Only add the set if it has at least one marker
if foundMarkers.Len() > 0 {
conflictingMarkers = append(conflictingMarkers, foundMarkers)
}
}

// If two or more sets have markers, report the conflict
if len(conflictingMarkers) >= 2 {
reportConflict(pass, field, conflictSet, conflictingMarkers)
}
}

func reportConflict(pass *analysis.Pass, field *ast.Field, conflictSet ConflictSet, conflictingMarkers []sets.Set[string]) {
// Build a descriptive message showing which sets conflict
setDescriptions := make([]string, 0, len(conflictingMarkers))

for _, set := range conflictingMarkers {
markersList := sets.List(set)
setDescriptions = append(setDescriptions, fmt.Sprintf("%v", markersList))
}

message := fmt.Sprintf("field %s has conflicting markers: %s: {%s}. %s",
field.Names[0].Name,
conflictSet.Name,
strings.Join(setDescriptions, ", "),
conflictSet.Description)

pass.Report(analysis.Diagnostic{
Pos: field.Pos(),
Message: message,
})
}
51 changes: 51 additions & 0 deletions pkg/analysis/conflictingmarkers/analyzer_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
Copyright 2025 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package conflictingmarkers_test

import (
"testing"

"golang.org/x/tools/go/analysis/analysistest"
"sigs.k8s.io/kube-api-linter/pkg/analysis/conflictingmarkers"
)

func TestConflictingMarkersAnalyzer(t *testing.T) {
testdata := analysistest.TestData()

config := &conflictingmarkers.ConflictingMarkersConfig{
Conflicts: []conflictingmarkers.ConflictSet{
{
Name: "test_conflict",
Sets: [][]string{{"marker1", "marker2"}, {"marker3", "marker4"}},
Description: "Test markers conflict with each other",
},
{
Name: "three_way_conflict",
Sets: [][]string{{"marker5", "marker6"}, {"marker7", "marker8"}, {"marker9", "marker10"}},
Description: "Three-way conflict between marker sets",
},
},
}

initializer := conflictingmarkers.Initializer()

analyzer, err := initializer.Init(config)
if err != nil {
t.Fatal(err)
}

analysistest.Run(t, testdata, analyzer, "a")
}
39 changes: 39 additions & 0 deletions pkg/analysis/conflictingmarkers/config.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
Copyright 2025 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package conflictingmarkers

// ConflictingMarkersConfig contains the configuration for the conflictingmarkers linter.
type ConflictingMarkersConfig struct {
// Conflicts allows users to define sets of conflicting markers.
// Each entry defines a conflict between multiple sets of markers.
Conflicts []ConflictSet `json:"conflicts"`
}

// ConflictSet represents a conflict between multiple sets of markers.
// Markers within each set are mutually exclusive with markers in all other sets.
// The linter will emit a diagnostic when a field has markers from two or more sets.
type ConflictSet struct {
// Name is a human-readable name for this conflict set.
// This name will appear in diagnostic messages to identify the type of conflict.
Name string `json:"name"`
// Sets contains the sets of markers that are mutually exclusive with each other.
// Each set is a slice of marker identifiers.
// The linter will emit a diagnostic when a field has markers from two or more sets.
Sets [][]string `json:"sets"`
// Description provides a description of why these markers conflict.
// The linter will include this description in the diagnostic message when a conflict is detected.
Description string `json:"description"`
}
29 changes: 29 additions & 0 deletions pkg/analysis/conflictingmarkers/conflictingmarkers_suite_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
Copyright 2025 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package conflictingmarkers_test

import (
"testing"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

func TestConflictingMarkers(t *testing.T) {
RegisterFailHandler(Fail)
RunSpecs(t, "conflictingmarkers")
}
60 changes: 60 additions & 0 deletions pkg/analysis/conflictingmarkers/doc.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/*
Copyright 2025 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

/*
conflictingmarkers is a linter that detects and reports when mutually exclusive markers are used on the same field.
This prevents common configuration errors and unexpected behavior in Kubernetes API types.

The linter reports issues when markers from two or more sets of a conflict definition are present on the same field.
It does NOT report issues when multiple markers from the same set are present - only when markers from
different sets within the same conflict definition are found together.

The linter is fully configurable and requires users to define all conflict sets they want to check.
There are no built-in conflict sets - all conflicts must be explicitly configured.

Each conflict set must specify:
- A unique name for the conflict
- Multiple sets of markers that are mutually exclusive with each other (at least 2 sets)
- A description explaining why the markers conflict

Example configuration:
```yaml
lintersConfig:

conflictingmarkers:
conflicts:
- name: "optional_vs_required"
sets:
- ["optional", "+kubebuilder:validation:Optional", "+k8s:validation:optional"]
- ["required", "+kubebuilder:validation:Required", "+k8s:validation:required"]
description: "A field cannot be both optional and required"
- name: "my_custom_conflict"
sets:
- ["custom:marker1", "custom:marker2"]
- ["custom:marker3", "custom:marker4"]
- ["custom:marker5", "custom:marker6"]
description: "These markers define different storage backends that cannot be used simultaneously"

```

Configuration options:
- `conflicts`: Required list of conflict set definitions.

Note: This linter is not enabled by default and must be explicitly enabled in the configuration.

The linter does not provide automatic fixes as it cannot determine which conflicting marker should be removed.
*/
package conflictingmarkers
Loading