-
Notifications
You must be signed in to change notification settings - Fork 21
Add references linter #163
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
4e54c0c
4807f4b
7f06ecd
2502e96
c5a8a96
84e1d3f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,6 +31,7 @@ linters: | |
| - govet | ||
| - importas | ||
| - ineffassign | ||
| - kubeapilinter | ||
| - makezero | ||
| - misspell | ||
| - nakedret | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,7 @@ | |
| - [Notimestamp](#notimestamp) - Prevents usage of 'TimeStamp' fields | ||
| - [OptionalFields](#optionalfields) - Validates optional field conventions | ||
| - [OptionalOrRequired](#optionalorrequired) - Ensures fields are explicitly marked as optional or required | ||
| - [References](#references) - Ensures field names use Ref/Refs instead of Reference/References | ||
| - [RequiredFields](#requiredfields) - Validates required field conventions | ||
| - [SSATags](#ssatags) - Ensures proper Server-Side Apply (SSA) tags on array fields | ||
| - [StatusOptional](#statusoptional) - Ensures status fields are marked as optional | ||
|
|
@@ -555,6 +556,38 @@ If you prefer not to suggest fixes for pointers in required fields, you can chan | |
| If you prefer not to suggest fixes for `omitempty` in required fields, you can change the `omitempty.policy` to `Warn` or `Ignore`. | ||
| If you prefer not to suggest fixes for `omitzero` in required fields, you can change the `omitzero.policy` to `Warn` and also not to consider `omitzero` policy at all, it can be set to `Forbid`. | ||
|
|
||
| ## References | ||
|
|
||
| The `references` linter ensures that field names use 'Ref'/'Refs' suffixes instead of 'Reference'/'References'. | ||
|
|
||
| By default, `references` is enabled and enforces this naming convention. The linter checks that: | ||
| - Fields ending with 'Reference' are suggested to use 'Ref' instead | ||
| - Fields ending with 'References' are suggested to use 'Refs' instead | ||
|
|
||
| ### Configuration | ||
|
|
||
| ```yaml | ||
| lintersConfig: | ||
| references: | ||
| allowRefAndRefs: false | true # Allow Ref/Refs suffixes for OpenShift compatibility. Defaults to `false`. | ||
|
||
| ``` | ||
|
|
||
| **Default behavior (allowRefAndRefs=false):** | ||
| - Reports errors for fields ending with 'Reference' or 'References' | ||
| - Also reports errors for fields ending with 'Ref' or 'Refs' (unless they were corrected from Reference/References) | ||
|
||
|
|
||
| **OpenShift compatibility (allowRefAndRefs=true):** | ||
|
||
| - Reports errors for fields ending with 'Reference' or 'References' | ||
| - Allows fields ending with 'Ref' or 'Refs' without reporting errors | ||
|
||
|
|
||
| ### Fixes | ||
|
|
||
| The `references` linter can automatically fix field names by replacing 'Reference' with 'Ref' and 'References' with 'Refs'. | ||
|
|
||
| For example: | ||
| - `NodeReference` will be suggested to be changed to `NodeRef` | ||
| - `NodeReferences` will be suggested to be changed to `NodeRefs` | ||
|
|
||
| ## SSATags | ||
|
|
||
| The `ssatags` linter ensures that array fields in Kubernetes API objects have the appropriate | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,131 @@ | ||
| /* | ||
| 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 references | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "go/ast" | ||
| "strings" | ||
|
|
||
| "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 = "references" | ||
|
|
||
| type analyzer struct { | ||
| allowRefAndRefs bool | ||
| } | ||
|
|
||
| // newAnalyzer creates a new analysis.Analyzer for the references linter. | ||
| func newAnalyzer(cfg *Config) *analysis.Analyzer { | ||
| if cfg == nil { | ||
| cfg = &Config{} | ||
| } | ||
|
|
||
| a := &analyzer{ | ||
| allowRefAndRefs: cfg.AllowRefAndRefs, | ||
| } | ||
|
|
||
| analyzer := &analysis.Analyzer{ | ||
| Name: name, | ||
| Doc: "Enforces that fields use Ref/Refs and not Reference/References", | ||
| Run: a.run, | ||
| Requires: []*analysis.Analyzer{inspector.Analyzer, extractjsontags.Analyzer}, | ||
| } | ||
|
|
||
| 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, jsonTagInfo extractjsontags.FieldTagInfo, markersAccess markers.Markers) { | ||
| a.checkField(pass, field, jsonTagInfo) | ||
| }) | ||
|
|
||
| return nil, nil //nolint:nilnil | ||
| } | ||
|
|
||
| func (a *analyzer) checkField(pass *analysis.Pass, field *ast.Field, jsonTagInfo extractjsontags.FieldTagInfo) { | ||
| if field == nil || len(field.Names) == 0 { | ||
| return | ||
| } | ||
|
|
||
| fieldName := utils.FieldName(field) | ||
|
|
||
| if strings.HasSuffix(fieldName, "Reference") { | ||
| suggestedName := strings.TrimSuffix(fieldName, "Reference") + "Ref" | ||
| pass.Report(analysis.Diagnostic{ | ||
| Pos: field.Pos(), | ||
| Message: fmt.Sprintf("field %s should use 'Ref' instead of 'Reference'", fieldName), | ||
| SuggestedFixes: []analysis.SuggestedFix{ | ||
| { | ||
| Message: "replace 'Reference' with 'Ref'", | ||
| TextEdits: []analysis.TextEdit{ | ||
| { | ||
| Pos: field.Names[0].Pos(), | ||
| NewText: []byte(suggestedName), | ||
| End: field.Names[0].End(), | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }) | ||
| return | ||
| } | ||
|
|
||
| if strings.HasSuffix(fieldName, "References") { | ||
| suggestedName := strings.TrimSuffix(fieldName, "References") + "Refs" | ||
| pass.Report(analysis.Diagnostic{ | ||
| Pos: field.Pos(), | ||
| Message: fmt.Sprintf("field %s should use 'Refs' instead of 'References'", fieldName), | ||
| SuggestedFixes: []analysis.SuggestedFix{ | ||
| { | ||
| Message: "replace 'References' with 'Refs'", | ||
| TextEdits: []analysis.TextEdit{ | ||
| { | ||
| Pos: field.Names[0].Pos(), | ||
| NewText: []byte(suggestedName), | ||
| End: field.Names[0].End(), | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }) | ||
| return | ||
| } | ||
|
|
||
| // If allowRefAndRefs is false, report errors for standalone Ref/Refs fields | ||
| // If allowRefAndRefs is true (OpenShift), don't report errors for Ref/Refs fields | ||
| if !a.allowRefAndRefs { | ||
| // Check for fields ending with Ref or Refs (excluding those already handled above) | ||
| if fieldName != "Ref" && strings.HasSuffix(fieldName, "Ref") && !strings.HasSuffix(fieldName, "eRef") { | ||
| pass.Reportf(field.Pos(), "field %s should not use 'Ref' suffix", fieldName) | ||
| } | ||
|
|
||
| if fieldName != "Refs" && strings.HasSuffix(fieldName, "Refs") && !strings.HasSuffix(fieldName, "eRefs") { | ||
| pass.Reportf(field.Pos(), "field %s should not use 'Refs' suffix", fieldName) | ||
| } | ||
|
||
| } | ||
| } | ||
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| /* | ||
| 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 references_test | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "golang.org/x/tools/go/analysis/analysistest" | ||
| "sigs.k8s.io/kube-api-linter/pkg/analysis/references" | ||
| ) | ||
|
|
||
| func TestDefaultConfiguration(t *testing.T) { | ||
| testdata := analysistest.TestData() | ||
|
|
||
| cfg := &references.Config{ | ||
| AllowRefAndRefs: true, | ||
| } | ||
JoelSpeed marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| analyzer, err := references.Initializer().Init(cfg) | ||
| if err != nil { | ||
| t.Fatalf("initializing references linter: %v", err) | ||
| } | ||
|
|
||
| analysistest.RunWithSuggestedFixes(t, testdata, analyzer, "a") | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| /* | ||
| 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 references | ||
|
|
||
| // Config represents the configuration for the references linter. | ||
| type Config struct { | ||
| // AllowRefAndRefs, when set to true, allows fields ending with "Ref" or "Refs". | ||
| // This is useful for OpenShift compatibility where Ref/Refs fields are used instead of Reference/References. | ||
| // When false (default), the linter will report errors for all fields ending with "Ref" or "Refs". | ||
| AllowRefAndRefs bool `json:"allowRefAndRefs,omitempty"` | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| /* | ||
| 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 `references` linter ensures that field names use 'Ref'/'Refs' suffixes and not 'Reference'/'References'. | ||
|
|
||
| By default, `references` is enabled and enforces this naming convention. | ||
|
|
||
| The linter checks that fields ending with 'Reference' are suggested to use 'Ref' instead. | ||
| Similarly, fields ending with 'References' are suggested to use 'Refs' instead. | ||
|
|
||
| Example configuration: | ||
|
|
||
| **Default behavior (report errors for Reference/References):** | ||
| ```yaml | ||
| lintersConfig: | ||
|
|
||
| references: {} | ||
|
|
||
| ``` | ||
|
|
||
| **For OpenShift compatibility (allow Ref/Refs):** | ||
| ```yaml | ||
| lintersConfig: | ||
|
|
||
| references: | ||
| allowRefAndRefs: true | ||
|
|
||
| ``` | ||
|
|
||
| When `allowRefAndRefs` is set to false (the default), fields ending with 'Ref' or 'Refs' (other than those matched by the above rules) will also be reported as errors. | ||
| This is useful to ensure consistency across the codebase. However, for OpenShift compatibility, this option can be set to true to allow such field names. | ||
| */ | ||
| package references |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| /* | ||
| 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 references | ||
|
|
||
| import ( | ||
| "golang.org/x/tools/go/analysis" | ||
| "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, | ||
| true, | ||
| validateConfig, | ||
| ) | ||
| } | ||
|
|
||
| func initAnalyzer(cfg *Config) (*analysis.Analyzer, error) { | ||
| return newAnalyzer(cfg), nil | ||
| } | ||
|
|
||
| // validateConfig validates the configuration for the references linter. | ||
| func validateConfig(cfg *Config, fldPath *field.Path) field.ErrorList { | ||
| // No validation needed for this simple config | ||
|
||
| return nil | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh? 👀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverted that change 👍🏻 can't have kubeapilinter as a linter itself for this project 😆
Was testing out/exploring different files in the linter and so might've committed this as a change too