Skip to content

linters: add forbiddenmarkers and nonullable linters #111

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
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
70 changes: 69 additions & 1 deletion docs/linters.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@
- [Conditions](#conditions) - Checks that `Conditions` fields are correctly formatted
- [CommentStart](#commentstart) - Ensures comments start with the serialized form of the type
- [DuplicateMarkers](#duplicatemarkers) - Checks for exact duplicates of markers
- [ForbiddenMarkers](#forbiddenmarkers) - Checks that no forbidden markers are present on types/fields.
- [Integers](#integers) - Validates usage of supported integer types
- [JSONTags](#jsontags) - Ensures proper JSON tag formatting
- [MaxLength](#maxlength) - Checks for maximum length constraints on strings and arrays
- [NoBools](#nobools) - Prevents usage of boolean types
- [NoFloats](#nofloats) - Prevents usage of floating-point types
- [Nomaps](#nomaps) - Restricts usage of map types
- [NoNullable](#nonullable) - Prevents usage of the nullable marker
- [Nophase](#nophase) - Prevents usage of 'Phase' fields
- [Notimestamp](#notimestamp) - Prevents usage of 'TimeStamp' fields
- [OptionalFields](#optionalfields) - Validates optional field conventions
Expand Down Expand Up @@ -98,6 +100,62 @@ will not.
The `duplicatemarkers` linter can automatically fix all markers that are exact match to another markers.
If there are duplicates across fields and their underlying type, the marker on the type will be preferred and the marker on the field will be removed.

## ForbiddenMarkers

The `forbiddenmarkers` linter ensures that types and fields do not contain any markers that are forbidden.

By default, `forbiddenmarkers` is not enabled.

### Configuation

It can be configured with a list of marker identifiers and optionally their attributes and values that are forbidden

```yaml
lintersConfig:
forbiddenMarkers:
markers:
- identifier: forbidden:marker
- identifier: forbidden:withAttribute
attributes:
- name: fruit
- identifier: forbidden:withMultipleAttributes
attributes:
- name: fruit
- name: color
- identifier: forbidden:withAttributeValues
attributes:
- name: fruit
values:
- apple
- banana
- orange
- identifier: forbidden:withMultipleAttributesValues
attributes:
- name: fruit
values:
- apple
- banana
- orange
- name: color
values:
- red
- green
- blue
```

Using the config above, the following examples would be forbidden:

- `+forbidden:marker` (all instances, including if they have attributes and values)
- `+forbidden:withAttribute:fruit=*,*` (all instances of this marker containing the attribute 'fruit')
- `+forbidden:withMultipleAttributes:fruit=*,color=*,*` (all instances of this marker containing both the 'fruit' AND 'color' attributes)
- `+forbidden:withAttributeValues:fruit={apple || banana || orange},*` (all instances of this marker containing the 'fruit' attribute where the value is set to one of 'apple', 'banana', or 'orange')

- `+forbidden:withMultipleAttributesValues:fruit={apple || banana || orange},color={red || green || blue},*` (all instances of this marker containing the 'fruit' attribute where the value is set to one of 'apple', 'banana', or 'orange' AND the 'color' attribute where the value is set to one of 'red', 'green', or 'blue')

### Fixes

Fixes are suggested to remove all markers that are forbidden.

## Integers

The `integers` linter checks for usage of unsupported integer types.
Expand Down Expand Up @@ -161,6 +219,14 @@ lintersConfig:
policy: Enforce | AllowStringToStringMaps | Ignore # Determines how the linter should handle maps of simple types. Defaults to AllowStringToStringMaps.
```

## NoNullable

The `nonullable` linter ensures that types and fields do not have the `nullable` marker.

### Fixes

Fixes are suggested to remove the `nullable` marker.

## Notimestamp

The `notimestamp` linter checks that the fields in the API are not named with the word 'Timestamp'.
Expand Down Expand Up @@ -334,10 +400,12 @@ linter will suggest adding the comment `// +kubebuilder:subresource:status` abov

The `uniquemarkers` linter ensures that types and fields do not contain more than a single definition of a marker that should only be present once.

Because this linter has no way of determining which marker definition was intended it does not suggest any fixes
Because this linter has no way of determining which marker definition was intended it does not suggest any fixes

### Configuration

It can configured to include a set of custom markers in the analysis by setting:

```yaml
lintersConfig:
uniquemarkers:
Expand Down
175 changes: 175 additions & 0 deletions pkg/analysis/forbiddenmarkers/analyzer.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
/*
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 forbiddenmarkers

import (
"fmt"
"go/ast"

"golang.org/x/tools/go/analysis"
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 = "forbiddenmarkers"

type analyzer struct {
forbiddenMarkers []Marker
}

// NewAnalyzer creates a new analysis.Analyzer for the forbiddenmarkers
// linter based on the provided config.ForbiddenMarkersConfig.
func newAnalyzer(cfg *Config) *analysis.Analyzer {
a := &analyzer{
forbiddenMarkers: cfg.Markers,
}

analyzer := &analysis.Analyzer{
Name: name,
Doc: "Check that no forbidden markers are present on types and fields.",
Run: a.run,
Requires: []*analysis.Analyzer{inspector.Analyzer},
}

for _, marker := range a.forbiddenMarkers {
markers.DefaultRegistry().Register(marker.Identifier)
}

return 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.forbiddenMarkers)
})

inspect.InspectTypeSpec(func(typeSpec *ast.TypeSpec, markersAccess markers.Markers) {
checkType(pass, typeSpec, markersAccess, a.forbiddenMarkers)
})

return nil, nil //nolint:nilnil
}

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

markers := utils.TypeAwareMarkerCollectionForField(pass, markersAccess, field)
check(markers, forbiddenMarkers, reportField(pass, field))
}

func checkType(pass *analysis.Pass, typeSpec *ast.TypeSpec, markersAccess markers.Markers, forbiddenMarkers []Marker) {
if typeSpec == nil {
return
}

markers := markersAccess.TypeMarkers(typeSpec)
check(markers, forbiddenMarkers, reportType(pass, typeSpec))
}

func check(markerSet markers.MarkerSet, forbiddenMarkers []Marker, reportFunc func(marker markers.Marker)) {
for _, marker := range forbiddenMarkers {
marks := markerSet.Get(marker.Identifier)
for _, mark := range marks {
if markerMatchesAttributeRules(mark, marker.Attributes...) {
reportFunc(mark)
}
}
}
}

func markerMatchesAttributeRules(marker markers.Marker, attrRules ...MarkerAttribute) bool {
matchesAll := true
for _, attrRule := range attrRules {

Check failure on line 105 in pkg/analysis/forbiddenmarkers/analyzer.go

View workflow job for this annotation

GitHub Actions / golangci-lint

ranges should only be cuddled with assignments used in the iteration (wsl)
if val, ok := marker.Expressions[attrRule.Name]; ok {
// if no values are specified, that means the existence match is enough
// and we can continue to the next rule
if len(attrRule.Values) == 0 {
continue
}

// if the value doesn't match one of the forbidden ones, this marker is not forbidden
matchesOneValue := false
for _, value := range attrRule.Values {

Check failure on line 115 in pkg/analysis/forbiddenmarkers/analyzer.go

View workflow job for this annotation

GitHub Actions / golangci-lint

ranges should only be cuddled with assignments used in the iteration (wsl)
if val == value {
matchesOneValue = true
break
}
}

if !matchesOneValue {
matchesAll = false
break
}
continue

Check failure on line 126 in pkg/analysis/forbiddenmarkers/analyzer.go

View workflow job for this annotation

GitHub Actions / golangci-lint

continue with no blank line before (nlreturn)
}
// if the marker doesn't contain the attribute for a specified rule it fails the AND
// operation.
matchesAll = false
break

Check failure on line 131 in pkg/analysis/forbiddenmarkers/analyzer.go

View workflow job for this annotation

GitHub Actions / golangci-lint

break with no blank line before (nlreturn)
}

return matchesAll
}

func reportField(pass *analysis.Pass, field *ast.Field) func(marker markers.Marker) {
return func(marker markers.Marker) {
pass.Report(analysis.Diagnostic{
Pos: field.Pos(),
Message: fmt.Sprintf("field %s has forbidden marker %q", field.Names[0].Name, marker.String()),
SuggestedFixes: []analysis.SuggestedFix{
{
Message: fmt.Sprintf("remove forbidden marker %q", marker.String()),
TextEdits: []analysis.TextEdit{
{
Pos: marker.Pos,
End: marker.End,
},
},
},
},
})
}
}

func reportType(pass *analysis.Pass, typeSpec *ast.TypeSpec) func(marker markers.Marker) {
return func(marker markers.Marker) {
pass.Report(analysis.Diagnostic{
Pos: typeSpec.Pos(),
Message: fmt.Sprintf("type %s has forbidden marker %q", typeSpec.Name, marker.String()),
SuggestedFixes: []analysis.SuggestedFix{
{
Message: fmt.Sprintf("remove forbidden marker %q", marker.String()),
TextEdits: []analysis.TextEdit{
{
Pos: marker.Pos,
End: marker.End,
},
},
},
},
})
}
}
86 changes: 86 additions & 0 deletions pkg/analysis/forbiddenmarkers/analyzer_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/*
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 forbiddenmarkers

import (
"testing"

"golang.org/x/tools/go/analysis/analysistest"
)

func TestWithConfiguration(t *testing.T) {
testdata := analysistest.TestData()
analysistest.RunWithSuggestedFixes(t, testdata, newAnalyzer(&Config{
Markers: []Marker{
{
Identifier: "custom:forbidden",
},
{
Identifier: "custom:AttrNoValues",
Attributes: []MarkerAttribute{
{
Name: "fruit",
},
},
},
{
Identifier: "custom:AttrValues",
Attributes: []MarkerAttribute{
{
Name: "fruit",
Values: []string{
"apple",
"orange",
"banana",
},
},
},
},
{
Identifier: "custom:AttrsNoValues",
Attributes: []MarkerAttribute{
{
Name: "fruit",
},
{
Name: "color",
},
},
},
{
Identifier: "custom:AttrsValues",
Attributes: []MarkerAttribute{
{
Name: "fruit",
Values: []string{
"apple",
"orange",
"banana",
},
},
{
Name: "color",
Values: []string{
"red",
"blue",
"green",
},
},
},
},
},
}), "a/...")
}
Loading
Loading