Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ linters:
- govet
- importas
- ineffassign
- kubeapilinter
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh? 👀

Copy link
Contributor Author

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

- makezero
- misspell
- nakedret
Expand Down
33 changes: 33 additions & 0 deletions docs/linters.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We generally prefer enums over bools, so maybe lets change this to be more like policy config like we have in other places. Perhaps the values could be AllowRefAndRefs and ForbidRefAndRefs?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Let's also drop the OpenShift mention. This is a vendor-neutral project.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes i agree. Having policy config in an enum would make a lot more sense. Incorporated in latest commit 👍🏻

```

**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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It says “suffix,” but does that mean it doesn't allow fields named Ref or Refs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed "suffix" as we are replacing instances of 'Reference' and 'References' anywhere across the field. beginning(prefix), middle, end


**OpenShift compatibility (allowRefAndRefs=true):**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OpenShift compatibility

Is this unique problem with OpenShift ? Other projects might use this option, so I suggest to rename this to more general.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not unique to Openshift. Removed mention 👍🏻

- Reports errors for fields ending with 'Reference' or 'References'
- Allows fields ending with 'Ref' or 'Refs' without reporting errors
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this mode we want to also report Ref and Refs and drop all of these


### 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
Expand Down
131 changes: 131 additions & 0 deletions pkg/analysis/references/analyzer.go
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)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is eRef and eRefs ? why do we check the field with it ? Isn't it enough to only check Ref or Refs ?
I want some examples for test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replaced the initially added condition to check for letters/patterns before or after. Incorporated edge cases in test files 👍🏻

}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is all very well written, but, take a look at the namingconventions linter. I believe we can implement this linter as a wrapper around that linter just by providing it with fixed configuration based on the config rules for this particular linter.

38 changes: 38 additions & 0 deletions pkg/analysis/references/analyzer_test.go
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,
}

analyzer, err := references.Initializer().Init(cfg)
if err != nil {
t.Fatalf("initializing references linter: %v", err)
}

analysistest.RunWithSuggestedFixes(t, testdata, analyzer, "a")
}
24 changes: 24 additions & 0 deletions pkg/analysis/references/config.go
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"`
}
47 changes: 47 additions & 0 deletions pkg/analysis/references/doc.go
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
48 changes: 48 additions & 0 deletions pkg/analysis/references/initializer.go
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we switch to an enum, I'd expect some validation here FWIW

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

policy validation incorporated 👍🏻

return nil
}
Loading