diff --git a/docs/linters.md b/docs/linters.md index 83e95c51..6d060143 100644 --- a/docs/linters.md +++ b/docs/linters.md @@ -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 @@ -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. @@ -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'. @@ -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: diff --git a/pkg/analysis/forbiddenmarkers/analyzer.go b/pkg/analysis/forbiddenmarkers/analyzer.go new file mode 100644 index 00000000..72897f30 --- /dev/null +++ b/pkg/analysis/forbiddenmarkers/analyzer.go @@ -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 { + 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 { + if val == value { + matchesOneValue = true + break + } + } + + if !matchesOneValue { + matchesAll = false + break + } + continue + } + // if the marker doesn't contain the attribute for a specified rule it fails the AND + // operation. + matchesAll = false + break + } + + 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, + }, + }, + }, + }, + }) + } +} diff --git a/pkg/analysis/forbiddenmarkers/analyzer_test.go b/pkg/analysis/forbiddenmarkers/analyzer_test.go new file mode 100644 index 00000000..e8011793 --- /dev/null +++ b/pkg/analysis/forbiddenmarkers/analyzer_test.go @@ -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/...") +} diff --git a/pkg/analysis/forbiddenmarkers/config.go b/pkg/analysis/forbiddenmarkers/config.go new file mode 100644 index 00000000..83baee67 --- /dev/null +++ b/pkg/analysis/forbiddenmarkers/config.go @@ -0,0 +1,43 @@ +package forbiddenmarkers + +// Config is the configuration type +// for the forbiddenmarkers linter. +type Config struct { + // markers is the unique set of markers + // that are forbidden on types/fields. + // Uniqueness is keyed on the `identifier` + // field of entries. + // Must have at least one entry. + Markers []Marker `json:"markers"` +} + +// Marker is a representation of a +// type/field marker that should be forbidden. +type Marker struct { + // identifier is the identifier for the forbidden marker. + Identifier string `json:"identifier"` + // attributes is an optional unique set of + // attributes that is forbidden for this marker. + // Uniqueness is keyed on the `name` field of entries. + // When specified, only instances of this marker + // that contains all the attributes will be considered + // forbidden. + // When not specified, any instance of the marker will be + // considered forbidden. + Attributes []MarkerAttribute `json:"attributes,omitempty"` +} + +// MarkerAttribute is a representation of an +// attribute for a marker. +type MarkerAttribute struct { + // name is the name of the forbidden attribute + Name string `json:"name"` + // values is an optional unique set of + // values that are forbidden for this marker. + // When specified, only the instances of this + // attribute that set one of these forbidden values + // will be considered forbidden. + // When not specified, any use of this attribute + // will be considered forbidden. + Values []string `json:"values,omitempty"` +} diff --git a/pkg/analysis/forbiddenmarkers/doc.go b/pkg/analysis/forbiddenmarkers/doc.go new file mode 100644 index 00000000..f83ce5d6 --- /dev/null +++ b/pkg/analysis/forbiddenmarkers/doc.go @@ -0,0 +1,67 @@ +/* +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. +*/ + +/* +* The `forbiddenmarkers` linter ensures that types and fields do not contain any markers +* that are forbidden. +* +* By default, `forbiddenmarkers` is not enabled. +* +* 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 are suggested to remove all markers that are forbidden. +* + */ +package forbiddenmarkers diff --git a/pkg/analysis/forbiddenmarkers/initializer.go b/pkg/analysis/forbiddenmarkers/initializer.go new file mode 100644 index 00000000..9501f5d0 --- /dev/null +++ b/pkg/analysis/forbiddenmarkers/initializer.go @@ -0,0 +1,129 @@ +/* +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 ( + "golang.org/x/tools/go/analysis" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/kube-api-linter/pkg/analysis/initializer" + "sigs.k8s.io/kube-api-linter/pkg/analysis/registry" +) + +func init() { + registry.DefaultRegistry().RegisterLinter(Initializer()) +} + +// Initializer returns the AnalyzerInitializer for this +// Analyzer so that it can be added to the registry. +func Initializer() initializer.AnalyzerInitializer { + return initializer.NewConfigurableInitializer( + name, + initAnalyzer, + false, + validateConfig, + ) +} + +func initAnalyzer(cfg *Config) (*analysis.Analyzer, error) { + return newAnalyzer(cfg), nil +} + +// validateConfig implements validation of the conditions linter config. +func validateConfig(cfg *Config, fldPath *field.Path) field.ErrorList { + if cfg == nil { + return field.ErrorList{field.Required(fldPath, "configuration is required for the forbiddenmarkers linter when it is enabled")} + } + + fieldErrors := field.ErrorList{} + + return fieldErrors +} + +func validateMarkers(fldPath *field.Path, markers ...Marker) field.ErrorList { + childPath := fldPath.Child("markers") + if len(markers) == 0 { + return field.ErrorList{field.Required(childPath, "must contain at least one forbidden marker")} + } + + fieldErrors := field.ErrorList{} + + knownMarkers := sets.New[string]() + + for i, marker := range markers { + indexPath := childPath.Index(i) + if knownMarkers.Has(marker.Identifier) { + fieldErrors = append(fieldErrors, field.Duplicate(indexPath, marker.Identifier)) + continue + } + + knownMarkers.Insert(marker.Identifier) + + fieldErrors = append(fieldErrors, validateAttributes(indexPath, marker.Attributes...)...) + } + + return fieldErrors +} + +func validateAttributes(fldPath *field.Path, attributes ...MarkerAttribute) field.ErrorList { + if len(attributes) == 0 { + return field.ErrorList{} + } + + childPath := fldPath.Child("attributes") + + fieldErrors := field.ErrorList{} + + knownAttributes := sets.New[string]() + + for i, attribute := range attributes { + indexPath := childPath.Index(i) + if knownAttributes.Has(attribute.Name) { + fieldErrors = append(fieldErrors, field.Duplicate(indexPath, attribute.Name)) + continue + } + + knownAttributes.Insert(attribute.Name) + + fieldErrors = append(fieldErrors, validateValues(indexPath, attribute.Values...)...) + } + + return fieldErrors +} + +func validateValues(fldPath *field.Path, values ...string) field.ErrorList { + if len(values) == 0 { + return field.ErrorList{} + } + + childPath := fldPath.Child("values") + + fieldErrors := field.ErrorList{} + + knownAttributes := sets.New[string]() + + for i, value := range values { + indexPath := childPath.Index(i) + if knownAttributes.Has(value) { + fieldErrors = append(fieldErrors, field.Duplicate(indexPath, value)) + continue + } + + knownAttributes.Insert(value) + } + + return fieldErrors +} diff --git a/pkg/analysis/forbiddenmarkers/initializer_test.go b/pkg/analysis/forbiddenmarkers/initializer_test.go new file mode 100644 index 00000000..d42356f3 --- /dev/null +++ b/pkg/analysis/forbiddenmarkers/initializer_test.go @@ -0,0 +1,146 @@ +/* +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_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/kube-api-linter/pkg/analysis/forbiddenmarkers" + "sigs.k8s.io/kube-api-linter/pkg/analysis/initializer" +) + +var _ = Describe("forbiddenmarkers initializer", func() { + Context("config validation", func() { + type testCase struct { + config *forbiddenmarkers.Config + expectedErr string + } + + DescribeTable("should validate the provided config", func(in testCase) { + ci, ok := forbiddenmarkers.Initializer().(initializer.ConfigurableAnalyzerInitializer) + Expect(ok).To(BeTrue()) + + errs := ci.ValidateConfig(in.config, field.NewPath("forbiddenmarkers")) + if len(in.expectedErr) > 0 { + Expect(errs.ToAggregate()).To(MatchError(in.expectedErr)) + } else { + Expect(errs).To(HaveLen(0), "No errors were expected") + } + }, + Entry("With a valid forbiddenmarkers configuration", testCase{ + config: &forbiddenmarkers.Config{ + Markers: []forbiddenmarkers.Marker{ + { + Identifier: "custom:forbidden", + }, + }, + }, + expectedErr: "", + }), + Entry("With an invalid forbiddenmarkers configuration, duplicate markers", testCase{ + config: &forbiddenmarkers.Config{ + Markers: []forbiddenmarkers.Marker{ + { + Identifier: "custom:forbidden", + }, + { + Identifier: "custom:forbidden", + }, + }, + }, + expectedErr: "invalid", + }), + Entry("With a valid forbiddenmarkers configuration with attributes", testCase{ + config: &forbiddenmarkers.Config{ + Markers: []forbiddenmarkers.Marker{ + { + Identifier: "custom:forbidden", + Attributes: []forbiddenmarkers.MarkerAttribute{ + { + Name: "fruit", + }, + }, + }, + }, + }, + expectedErr: "", + }), + Entry("With a valid forbiddenmarkers configuration with duplicate attributes", testCase{ + config: &forbiddenmarkers.Config{ + Markers: []forbiddenmarkers.Marker{ + { + Identifier: "custom:forbidden", + Attributes: []forbiddenmarkers.MarkerAttribute{ + { + Name: "fruit", + }, + { + Name: "fruit", + }, + }, + }, + }, + }, + expectedErr: "duplicate", + }), + Entry("With a valid forbiddenmarkers configuration with attributes with values", testCase{ + config: &forbiddenmarkers.Config{ + Markers: []forbiddenmarkers.Marker{ + { + Identifier: "custom:forbidden", + Attributes: []forbiddenmarkers.MarkerAttribute{ + { + Name: "fruit", + Values: []string{ + "apple", + "banana", + }, + }, + }, + }, + }, + }, + expectedErr: "", + }), + Entry("With an invalid forbiddenmarkers configuration with attributes with duplicate values", testCase{ + config: &forbiddenmarkers.Config{ + Markers: []forbiddenmarkers.Marker{ + { + Identifier: "custom:forbidden", + Attributes: []forbiddenmarkers.MarkerAttribute{ + { + Name: "fruit", + Values: []string{ + "apple", + "apple", + }, + }, + }, + }, + }, + }, + expectedErr: "duplicate", + }), + Entry("With a nil config", testCase{ + config: nil, + expectedErr: "required", + }), + ) + }) +}) diff --git a/pkg/analysis/forbiddenmarkers/testdata/src/a/a.go b/pkg/analysis/forbiddenmarkers/testdata/src/a/a.go new file mode 100644 index 00000000..003acb37 --- /dev/null +++ b/pkg/analysis/forbiddenmarkers/testdata/src/a/a.go @@ -0,0 +1,93 @@ +package a + +// +custom:forbidden +type ForbiddenMarkerType string // want `type ForbiddenMarkerType has forbidden marker "custom:forbidden"` + +// +custom:AttrNoValues:fruit=apple +type ForbiddenMarkerWithAttrType string // want `type ForbiddenMarkerWithAttrType has forbidden marker "custom:AttrNoValues:fruit=apple"` + +// +custom:AttrsNoValues:fruit=apple,color=blue +type ForbiddenMarkerWithMultipleAttrsType string // want `type ForbiddenMarkerWithMultipleAttrsType has forbidden marker "custom:AttrsNoValues:fruit=apple,color=blue"` + +// +custom:AttrValues:fruit=orange +type ForbiddenMarkerWithAttrWithValueType string // want `type ForbiddenMarkerWithAttrWithValueType has forbidden marker "custom:AttrValues:fruit=orange"` + +// +custom:AttrsValues:fruit=orange,color=blue +type ForbiddenMarkerWithAttrsWithValueType string // want `type ForbiddenMarkerWithAttrsWithValueType has forbidden marker "custom:AttrsValues:fruit=orange,color=blue"` + +// +allowed +type AllowedMarkerType string + +// Allowed because the configuration only forbids when the fruit attributes are specified +// +custom:AttrNoValues:color=blue +type AllowedMarkerWithAttrType string + +// Allowed because the configuration only forbids when both fruit and color attributes are specified +// +custom:AttrsNoValues:fruit=apple +type AllowedMarkerWithMultipleAttrsType string + +// Allowed because the configuration only forbids when the fruit attribute is one of apple, banana, or orange +// +custom:AttrsNoValues:fruit=cherry +type AllowedMarkerWithAttrWithValueType string + +// Allowed because the configuration only forbids when the fruit attribute is one of apple, banana, or orange +// and the color attribute is one of blue, red, or green +// +custom:AttrsValues:fruit=cherry,color=blue +type AllowedMarkerWithAttrsWithValueType string + +type Test struct { + // +custom:forbidden + ForbiddenMarkerField string `json:"forbiddenMarkerField"` // want `field ForbiddenMarkerField has forbidden marker "custom:forbidden"` + + ForbiddenMarkerFieldTypeAlias ForbiddenMarkerType `json:"forbiddenMarkerFieldTypeAlias"` // want `field ForbiddenMarkerFieldTypeAlias has forbidden marker "custom:forbidden"` + + // +custom:AttrNoValues:fruit=apple + ForbiddenMarkerWithAttrField string `json:"forbiddenMarkerWithAttrField"` // want `field ForbiddenMarkerWithAttrField has forbidden marker "custom:AttrNoValues:fruit=apple"` + + ForbiddenMarkerWithAttrFieldTypeAlias ForbiddenMarkerWithAttrType `json:"forbiddenMarkerWithAttrFieldTypeAlias"` // want `field ForbiddenMarkerWithAttrFieldTypeAlias has forbidden marker "custom:AttrNoValues:fruit=apple"` + + // +custom:AttrsNoValues:fruit=apple,color=blue + ForbiddenMarkerWithMultipleAttrsField string `json:"forbiddenMarkerWithMultipleAttrsField"` // want `field ForbiddenMarkerWithMultipleAttrsField has forbidden marker "custom:AttrsNoValues:fruit=apple,color=blue"` + + ForbiddenMarkerWithMutlipleAttrsFieldTypeAlias ForbiddenMarkerWithMultipleAttrsType `json:"forbiddenMarkerWithMultipleAttrsFieldTypeAlias"` // want `field ForbiddenMarkerWithMutlipleAttrsFieldTypeAlias has forbidden marker "custom:AttrsNoValues:fruit=apple,color=blue"` + + // +custom:AttrValues:fruit=orange + ForbiddenMarkerWithAttrWithValueField string `json:"forbiddenMarkerWithAttrWithValueField"` // want `field ForbiddenMarkerWithAttrWithValueField has forbidden marker "custom:AttrValues:fruit=orange"` + + ForbiddenMarkerWithAttrWithValueFieldTypeAlias ForbiddenMarkerWithAttrWithValueType `json:"forbiddenMarkerWithAttrWithValueFieldTypeAlias"` // want `field ForbiddenMarkerWithAttrWithValueFieldTypeAlias has forbidden marker "custom:AttrValues:fruit=orange"` + + // +custom:AttrsValues:fruit=orange,color=blue + ForbiddenMarkerWithAttrsWithValueField string `json:"forbiddenMarkerWithAttrsWithValueField"` // want `field ForbiddenMarkerWithAttrsWithValueField has forbidden marker "custom:AttrsValues:fruit=orange,color=blue"` + + ForbiddenMarkerWithAttrsWithValueFieldTypeAlias ForbiddenMarkerWithAttrsWithValueType `json:"forbiddenMarkerWithAttrsWithValueFieldTypeAlias"` // want `field ForbiddenMarkerWithAttrsWithValueFieldTypeAlias has forbidden marker "custom:AttrsValues:fruit=orange,color=blue"` + + // +allowed + AllowedMarkerField string `json:"allowedMarkerField"` + + AllowedMarkerFieldTypeAlias AllowedMarkerType `json:"allowedMarkerFieldTypeAlias"` + + // Allowed because the configuration only forbids when the fruit attributes are specified + // +custom:AttrNoValues:color=blue + AllowedMarkerWithAttrField string `json:"allowedMarkerWithAttrField"` + + AllowedMarkerWithAttrFieldTypeAlias AllowedMarkerWithAttrType `json:"allowedMarkerWithAttrFieldTypeAlias"` + + // Allowed because the configuration only forbids when both fruit and color attributes are specified + // +custom:AttrsNoValues:fruit=apple + AllowedMarkerWithMultipleAttrsField string `json:"allowedMarkerWithMultipleAttrsField"` + + AllowedMarkerWithMultipleAttrsFieldTypeAlias AllowedMarkerWithMultipleAttrsType `json:"allowedMarkerWithMultipleAttrsFieldTypeAlias"` + + // Allowed because the configuration only forbids when the fruit attribute is one of apple, banana, or orange + // +custom:AttrsNoValues:fruit=cherry + AllowedMarkerWithAttrWithValueField string `json:"allowedMarkerWithAttrWithValueField"` + + AllowedMarkerWithAttrWithValueFieldTypeAlias AllowedMarkerWithAttrWithValueType `json:"allowedMarkerWithAttrWithValueFieldTypeAlias"` + + // Allowed because the configuration only forbids when the fruit attribute is one of apple, banana, or orange + // and the color attribute is one of blue, red, or green + // +custom:AttrsValues:fruit=cherry,color=blue + AllowedMarkerWithAttrsWithValueField string `json:"allowedMarkerWithAttrsWithValueField"` + + AllowedMarkerWithAttrsWithValueFieldTypeAlias AllowedMarkerWithAttrsWithValueType `json:"allowedMarkerWithAttrsWithValueFieldTypeAlias"` +} diff --git a/pkg/analysis/forbiddenmarkers/testdata/src/a/a.go.golden b/pkg/analysis/forbiddenmarkers/testdata/src/a/a.go.golden new file mode 100644 index 00000000..d07f4817 --- /dev/null +++ b/pkg/analysis/forbiddenmarkers/testdata/src/a/a.go.golden @@ -0,0 +1,83 @@ +package a + +type ForbiddenMarkerType string // want `type ForbiddenMarkerType has forbidden marker "custom:forbidden"` + +type ForbiddenMarkerWithAttrType string // want `type ForbiddenMarkerWithAttrType has forbidden marker "custom:AttrNoValues:fruit=apple"` + +type ForbiddenMarkerWithMultipleAttrsType string // want `type ForbiddenMarkerWithMultipleAttrsType has forbidden marker "custom:AttrsNoValues:fruit=apple,color=blue"` + +type ForbiddenMarkerWithAttrWithValueType string // want `type ForbiddenMarkerWithAttrWithValueType has forbidden marker "custom:AttrValues:fruit=orange"` + +type ForbiddenMarkerWithAttrsWithValueType string // want `type ForbiddenMarkerWithAttrsWithValueType has forbidden marker "custom:AttrsValues:fruit=orange,color=blue"` + +// +allowed +type AllowedMarkerType string + +// Allowed because the configuration only forbids when the fruit attributes are specified +// +custom:AttrNoValues:color=blue +type AllowedMarkerWithAttrType string + +// Allowed because the configuration only forbids when both fruit and color attributes are specified +// +custom:AttrsNoValues:fruit=apple +type AllowedMarkerWithMultipleAttrsType string + +// Allowed because the configuration only forbids when the fruit attribute is one of apple, banana, or orange +// +custom:AttrsNoValues:fruit=cherry +type AllowedMarkerWithAttrWithValueType string + +// Allowed because the configuration only forbids when the fruit attribute is one of apple, banana, or orange +// and the color attribute is one of blue, red, or green +// +custom:AttrsValues:fruit=cherry,color=blue +type AllowedMarkerWithAttrsWithValueType string + +type Test struct { + ForbiddenMarkerField string `json:"forbiddenMarkerField"` // want `field ForbiddenMarkerField has forbidden marker "custom:forbidden"` + + ForbiddenMarkerFieldTypeAlias ForbiddenMarkerType `json:"forbiddenMarkerFieldTypeAlias"` // want `field ForbiddenMarkerFieldTypeAlias has forbidden marker "custom:forbidden"` + + ForbiddenMarkerWithAttrField string `json:"forbiddenMarkerWithAttrField"` // want `field ForbiddenMarkerWithAttrField has forbidden marker "custom:AttrNoValues:fruit=apple"` + + ForbiddenMarkerWithAttrFieldTypeAlias ForbiddenMarkerWithAttrType `json:"forbiddenMarkerWithAttrFieldTypeAlias"` // want `field ForbiddenMarkerWithAttrFieldTypeAlias has forbidden marker "custom:AttrNoValues:fruit=apple"` + + ForbiddenMarkerWithMultipleAttrsField string `json:"forbiddenMarkerWithMultipleAttrsField"` // want `field ForbiddenMarkerWithMultipleAttrsField has forbidden marker "custom:AttrsNoValues:fruit=apple,color=blue"` + + ForbiddenMarkerWithMutlipleAttrsFieldTypeAlias ForbiddenMarkerWithMultipleAttrsType `json:"forbiddenMarkerWithMultipleAttrsFieldTypeAlias"` // want `field ForbiddenMarkerWithMutlipleAttrsFieldTypeAlias has forbidden marker "custom:AttrsNoValues:fruit=apple,color=blue"` + + ForbiddenMarkerWithAttrWithValueField string `json:"forbiddenMarkerWithAttrWithValueField"` // want `field ForbiddenMarkerWithAttrWithValueField has forbidden marker "custom:AttrValues:fruit=orange"` + + ForbiddenMarkerWithAttrWithValueFieldTypeAlias ForbiddenMarkerWithAttrWithValueType `json:"forbiddenMarkerWithAttrWithValueFieldTypeAlias"` // want `field ForbiddenMarkerWithAttrWithValueFieldTypeAlias has forbidden marker "custom:AttrValues:fruit=orange"` + + ForbiddenMarkerWithAttrsWithValueField string `json:"forbiddenMarkerWithAttrsWithValueField"` // want `field ForbiddenMarkerWithAttrsWithValueField has forbidden marker "custom:AttrsValues:fruit=orange,color=blue"` + + ForbiddenMarkerWithAttrsWithValueFieldTypeAlias ForbiddenMarkerWithAttrsWithValueType `json:"forbiddenMarkerWithAttrsWithValueFieldTypeAlias"` // want `field ForbiddenMarkerWithAttrsWithValueFieldTypeAlias has forbidden marker "custom:AttrsValues:fruit=orange,color=blue"` + + // +allowed + AllowedMarkerField string `json:"allowedMarkerField"` + + AllowedMarkerFieldTypeAlias AllowedMarkerType `json:"allowedMarkerFieldTypeAlias"` + + // Allowed because the configuration only forbids when the fruit attributes are specified + // +custom:AttrNoValues:color=blue + AllowedMarkerWithAttrField string `json:"allowedMarkerWithAttrField"` + + AllowedMarkerWithAttrFieldTypeAlias AllowedMarkerWithAttrType `json:"allowedMarkerWithAttrFieldTypeAlias"` + + // Allowed because the configuration only forbids when both fruit and color attributes are specified + // +custom:AttrsNoValues:fruit=apple + AllowedMarkerWithMultipleAttrsField string `json:"allowedMarkerWithMultipleAttrsField"` + + AllowedMarkerWithMultipleAttrsFieldTypeAlias AllowedMarkerWithMultipleAttrsType `json:"allowedMarkerWithMultipleAttrsFieldTypeAlias"` + + // Allowed because the configuration only forbids when the fruit attribute is one of apple, banana, or orange + // +custom:AttrsNoValues:fruit=cherry + AllowedMarkerWithAttrWithValueField string `json:"allowedMarkerWithAttrWithValueField"` + + AllowedMarkerWithAttrWithValueFieldTypeAlias AllowedMarkerWithAttrWithValueType `json:"allowedMarkerWithAttrWithValueFieldTypeAlias"` + + // Allowed because the configuration only forbids when the fruit attribute is one of apple, banana, or orange + // and the color attribute is one of blue, red, or green + // +custom:AttrsValues:fruit=cherry,color=blue + AllowedMarkerWithAttrsWithValueField string `json:"allowedMarkerWithAttrsWithValueField"` + + AllowedMarkerWithAttrsWithValueFieldTypeAlias AllowedMarkerWithAttrsWithValueType `json:"allowedMarkerWithAttrsWithValueFieldTypeAlias"` +} diff --git a/pkg/analysis/nonullable/analyzer.go b/pkg/analysis/nonullable/analyzer.go new file mode 100644 index 00000000..5c2f9293 --- /dev/null +++ b/pkg/analysis/nonullable/analyzer.go @@ -0,0 +1,57 @@ +/* +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 nonullable + +import ( + "fmt" + + "golang.org/x/tools/go/analysis" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/kube-api-linter/pkg/analysis/forbiddenmarkers" + "sigs.k8s.io/kube-api-linter/pkg/analysis/initializer" +) + +const ( + name = "nonullable" + doc = "Check that nullable marker is not present on any types or fields." +) + +func newAnalyzer() *analysis.Analyzer { + cfg := &forbiddenmarkers.Config{ + Markers: []forbiddenmarkers.Marker{ + { + Identifier: "nullable", + }, + }, + } + + configInit := forbiddenmarkers.Initializer().(initializer.ConfigurableAnalyzerInitializer) + + errs := configInit.ValidateConfig(cfg, field.NewPath("nullable")) + if err := errs.ToAggregate(); err != nil { + panic(fmt.Errorf("nonullable linter has an invalid forbiddenmarkers configuration: %w", err)) + } + + analyzer, err := configInit.Init(cfg) + if err != nil { + panic(fmt.Errorf("nonullable linter encountered an error initializing wrapped forbiddenmarkers analyzer: %w", err)) + } + + analyzer.Name = name + analyzer.Doc = doc + + return analyzer +} diff --git a/pkg/analysis/nonullable/analyzer_test.go b/pkg/analysis/nonullable/analyzer_test.go new file mode 100644 index 00000000..7f1b78a8 --- /dev/null +++ b/pkg/analysis/nonullable/analyzer_test.go @@ -0,0 +1,27 @@ +/* +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 nonullable + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" +) + +func Test(t *testing.T) { + testdata := analysistest.TestData() + analysistest.RunWithSuggestedFixes(t, testdata, newAnalyzer(), "a/...") +} diff --git a/pkg/analysis/nonullable/doc.go b/pkg/analysis/nonullable/doc.go new file mode 100644 index 00000000..2b244c22 --- /dev/null +++ b/pkg/analysis/nonullable/doc.go @@ -0,0 +1,23 @@ +/* +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. +*/ + +/* +* The `nonullable` linter ensures that types and fields do not have the `nullable` marker. +* +* Fixes are suggested to remove the `nullable` marker. +* + */ +package nonullable diff --git a/pkg/analysis/nonullable/initializer.go b/pkg/analysis/nonullable/initializer.go new file mode 100644 index 00000000..d9e3b0b7 --- /dev/null +++ b/pkg/analysis/nonullable/initializer.go @@ -0,0 +1,35 @@ +/* +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 nonullable + +import ( + "sigs.k8s.io/kube-api-linter/pkg/analysis/initializer" + "sigs.k8s.io/kube-api-linter/pkg/analysis/registry" +) + +func init() { + registry.DefaultRegistry().RegisterLinter(Initializer()) +} + +// Initializer returns the AnalyzerInitializer for this +// Analyzer so that it can be added to the registry. +func Initializer() initializer.AnalyzerInitializer { + return initializer.NewInitializer( + name, + newAnalyzer(), + true, + ) +} diff --git a/pkg/analysis/nonullable/testdata/src/a/a.go b/pkg/analysis/nonullable/testdata/src/a/a.go new file mode 100644 index 00000000..409dc5ed --- /dev/null +++ b/pkg/analysis/nonullable/testdata/src/a/a.go @@ -0,0 +1,20 @@ +package a + +// +nullable +type NullableMarkerType string // want `type NullableMarkerType has forbidden marker "nullable"` + +// +allowed +type AllowedMarkerType string + +type Test struct { + // +nullable + NullableMarkerField string `json:"nullableMarkerField"`// want `field NullableMarkerField has forbidden marker "nullable"` + + NullableMarkerFieldTypeAlias NullableMarkerType `json:"nullableMarkerFieldTypeAlias"` // want `field NullableMarkerFieldTypeAlias has forbidden marker "nullable"` + + // +allowed + AllowedMarkerField string `json:"allowedMarkerField"` + + AllowedMarkerFieldTypeAlias AllowedMarkerType `json:"AllowedMarkerFieldTypeAlias"` +} + diff --git a/pkg/analysis/nonullable/testdata/src/a/a.go.golden b/pkg/analysis/nonullable/testdata/src/a/a.go.golden new file mode 100644 index 00000000..24beb282 --- /dev/null +++ b/pkg/analysis/nonullable/testdata/src/a/a.go.golden @@ -0,0 +1,18 @@ +package a + +type NullableMarkerType string // want `type NullableMarkerType has forbidden marker "nullable"` + +// +allowed +type AllowedMarkerType string + +type Test struct { + NullableMarkerField string `json:"nullableMarkerField"`// want `field NullableMarkerField has forbidden marker "nullable"` + + NullableMarkerFieldTypeAlias NullableMarkerType `json:"nullableMarkerFieldTypeAlias"` // want `field NullableMarkerFieldTypeAlias has forbidden marker "nullable"` + + // +allowed + AllowedMarkerField string `json:"allowedMarkerField"` + + AllowedMarkerFieldTypeAlias AllowedMarkerType `json:"AllowedMarkerFieldTypeAlias"` +} +