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
42 changes: 42 additions & 0 deletions pkg/analysis/arrayofstruct/analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@ package arrayofstruct
import (
"fmt"
"go/ast"
"regexp"
"slices"

"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"
Expand Down Expand Up @@ -74,6 +77,13 @@ func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markershelp
return
}

// Check if the struct has union markers that satisfy the required constraint
if hasExactlyOneOfMarker(structType) {
// ExactlyOneOf marker enforces that exactly one field is set,
// so we don't need to report an error
return
}

// Check if at least one field in the struct has a required marker
if hasRequiredField(structType, markersAccess) {
return
Expand Down Expand Up @@ -208,3 +218,35 @@ func hasRequiredField(structType *ast.StructType, markersAccess markershelper.Ma

return false
}

// hasExactlyOneOfMarker checks if the struct has an ExactlyOneOf marker,
// which satisfies the required field constraint by ensuring exactly one field is set.
func hasExactlyOneOfMarker(structType *ast.StructType) bool {
if structType.Fields == nil {
return false
}

for _, field := range structType.Fields.List {
Copy link
Contributor

Choose a reason for hiding this comment

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

Reading the description of this marker on https://book.kubebuilder.io/reference/markers/crd-validation it looks like this is only on the type definition and not the individual fields.

You should be able to get the set of markers on a struct using

func (m *markers) StructMarkers(sTyp *ast.StructType) MarkerSet {

Copy link
Author

Choose a reason for hiding this comment

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

Reading the description of this marker on https://book.kubebuilder.io/reference/markers/crd-validation it looks like this is only on the type definition and not the individual fields.

You should be able to get the set of markers on a struct using

func (m *markers) StructMarkers(sTyp *ast.StructType) MarkerSet {

Updated to use StructMarkers() and Marker is on struct definition

var markers []string

if field.Doc != nil {
for _, comment := range field.Doc.List {
markers = append(markers, comment.Text)
}
}
// Check for ExactlyOneOf marker
if hasMarkerPattern(markers, "ExactlyOneOf") {
return true
}
}

return false
}

// hasMarkerPattern checks if any of the markers match the given pattern.
func hasMarkerPattern(markers []string, markerName string) bool {
pattern := fmt.Sprintf(`\+kubebuilder:validation:%s=`, markerName)
re := regexp.MustCompile(pattern)

return slices.ContainsFunc(markers, re.MatchString)
}
12 changes: 12 additions & 0 deletions pkg/analysis/arrayofstruct/testdata/src/a/a.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,3 +150,15 @@ type ValidStructWithCustomBasicType struct {
// This should not trigger the linter because CustomString is based on string, a basic type
Items []CustomString
}

// Valid case - struct with ExactlyOneOf marker
type ValidWithExactlyOneOf struct {
Items []ValidExactlyOneOfItem
}

type ValidExactlyOneOfItem struct {
// +kubebuilder:validation:ExactlyOneOf=FieldA;FieldB
Copy link
Contributor

Choose a reason for hiding this comment

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

based on the documentation, i also see +kubebuilder:validation:AtMostOneOf
would we in future also consider that for completeness? @priyansh3006

Copy link
Author

Choose a reason for hiding this comment

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

@krishagarwal278 AtMostOneOf allows all fields to be empty, which goes against what this analyzer prevents. https://github.com/kubernetes-sigs/kube-api-linter/blob/main/pkg/analysis/arrayofstruct/analyzer.go#L34-L38

Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't seem like a valid use of this marker.

Copy link
Author

Choose a reason for hiding this comment

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

This doesn't seem like a valid use of this marker.

Sorry about that, Moved the marker to the struct type definition in the test:

// No required fields, but the marker enforces exactly one is set
FieldA *string `json:"fieldA,omitempty"`
FieldB *string `json:"fieldB,omitempty"`
}