Merge pull request #85 from sivchari/ssatags-linter

Introduce SSATags linter

Author: k8s-ci-robot

docs/linters.md

@@ -13,6 +13,7 @@
 - [OptionalFields](#optionalfields) - Validates optional field conventions
 - [OptionalOrRequired](#optionalorrequired) - Ensures fields are explicitly marked as optional or required
 - [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
 - [StatusSubresource](#statussubresource) - Validates status subresource configuration
 - [UniqueMarkers](#uniquemarkers) - Ensures unique marker definitions
@@ -248,6 +249,45 @@ It will suggest to remove the pointer from the field, and update the `json` tag
 If you prefer not to suggest fixes for pointers in required fields, you can change the `pointerPolicy` to `Warn`.
 The linter will then only suggest to remove the `omitempty` value from the `json` tag.
 
+## SSATags
+
+The `ssatags` linter ensures that array fields in Kubernetes API objects have the appropriate
+listType markers (atomic, set, or map) for proper Server-Side Apply behavior.
+
+Server-Side Apply (SSA) is a Kubernetes feature that allows multiple controllers to manage
+different parts of an object. The listType markers help SSA understand how to merge arrays:
+
+- listType=atomic: The entire list is replaced when updated
+- listType=set: List elements are treated as a set (no duplicates, order doesn't matter)
+- listType=map: Elements are identified by specific key fields for granular updates
+
+**Important Note on listType=set:**
+The use of listType=set is discouraged for object arrays due to Server-Side Apply
+compatibility issues. When multiple controllers attempt to apply changes to an object
+array with listType=set, the merge behavior can be unpredictable and may lead to
+data loss or unexpected conflicts. For object arrays, use listType=atomic for simple
+replacement semantics or listType=map for granular field-level merging.
+listType=set is safe to use with primitive arrays (strings, integers, etc.).
+
+The linter checks for:
+
+1. Missing listType markers on array fields
+2. Invalid listType values (must be atomic, set, or map)
+3. Usage of listType=set on object arrays (discouraged due to compatibility issues)
+4. Missing listMapKey markers for listType=map arrays
+5. Incorrect usage of listType=map on primitive arrays
+
+### Configuration
+
+```yaml
+lintersConfig:
+  ssaTags:
+    listTypeSetUsage: Warn | Ignore # The policy for listType=set usage on object arrays. Defaults to `Warn`.
+```
+
+**Note:** listMapKey validation is always enforced and cannot be disabled. This ensures proper
+Server-Side Apply behavior for arrays using listType=map.
+
 ## StatusOptional
 
 The `statusoptional` linter checks that all first-level children fields within a status struct are marked as optional.
@@ -304,4 +344,4 @@ Taking the example configuration from above:
 - Marker definitions of `custom:SomeCustomMarker:fruit=apple,color=red` and `custom:SomeCustomMarker:fruit=apple,color=green` would violate the uniqueness requirement and be flagged.
 - Marker definitions of `custom:SomeCustomMarker:fruit=apple,color=red` and `custom:SomeCustomMarker:fruit=orange,color=red` would _not_ violate the uniqueness requirement.
 
-Each entry in `customMarkers` must have a unique `identifier`.
+Each entry in `customMarkers` must have a unique `identifier`.

pkg/analysis/registry.go

@@ -32,6 +32,7 @@ import (
 	"sigs.k8s.io/kube-api-linter/pkg/analysis/optionalfields"
 	"sigs.k8s.io/kube-api-linter/pkg/analysis/optionalorrequired"
 	"sigs.k8s.io/kube-api-linter/pkg/analysis/requiredfields"
+	"sigs.k8s.io/kube-api-linter/pkg/analysis/ssatags"
 	"sigs.k8s.io/kube-api-linter/pkg/analysis/statusoptional"
 	"sigs.k8s.io/kube-api-linter/pkg/analysis/statussubresource"
 	"sigs.k8s.io/kube-api-linter/pkg/analysis/uniquemarkers"
@@ -89,6 +90,7 @@ func NewRegistry() Registry {
 			optionalfields.Initializer(),
 			optionalorrequired.Initializer(),
 			requiredfields.Initializer(),
+			ssatags.Initializer(),
 			statusoptional.Initializer(),
 			statussubresource.Initializer(),
 			uniquemarkers.Initializer(),

pkg/analysis/registry_test.go

@@ -42,6 +42,7 @@ var _ = Describe("Registry", func() {
 				"optionalfields",
 				"optionalorrequired",
 				"requiredfields",
+				"ssatags",
 				"statusoptional",
 				"uniquemarkers",
 			))
@@ -65,6 +66,7 @@ var _ = Describe("Registry", func() {
 				"optionalfields",
 				"optionalorrequired",
 				"requiredfields",
+				"ssatags",
 				"statusoptional",
 				"statussubresource",
 				"uniquemarkers",

pkg/analysis/ssatags/analyzer.go

@@ -0,0 +1,262 @@
+/*
+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 ssatags
+
+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"
+	"sigs.k8s.io/kube-api-linter/pkg/config"
+	kubebuildermarkers "sigs.k8s.io/kube-api-linter/pkg/markers"
+)
+
+const name = "ssatags"
+
+const (
+	listTypeAtomic = "atomic"
+	listTypeSet    = "set"
+	listTypeMap    = "map"
+)
+
+type analyzer struct {
+	cfg config.SSATagsConfig
+}
+
+func newAnalyzer(cfg config.SSATagsConfig) *analysis.Analyzer {
+	defaultConfig(&cfg)
+
+	a := &analyzer{
+		cfg: cfg,
+	}
+
+	return &analysis.Analyzer{
+		Name:     name,
+		Doc:      "Check that all array types in the API have a listType tag and the usage of the tags is correct",
+		Run:      a.run,
+		Requires: []*analysis.Analyzer{inspector.Analyzer, extractjsontags.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, markersAccess)
+	})
+
+	return nil, nil //nolint:nilnil
+}
+
+func (a *analyzer) checkField(pass *analysis.Pass, field *ast.Field, markersAccess markers.Markers) {
+	if !utils.IsArrayTypeOrAlias(pass, field) {
+		return
+	}
+
+	fieldMarkers := markersAccess.FieldMarkers(field)
+	if fieldMarkers == nil {
+		return
+	}
+
+	fieldName := utils.FieldName(field)
+	listTypeMarkers := fieldMarkers.Get(kubebuildermarkers.KubebuilderListTypeMarker)
+
+	if len(listTypeMarkers) == 0 {
+		pass.Report(analysis.Diagnostic{
+			Pos:     field.Pos(),
+			Message: fmt.Sprintf("%s should have a listType marker for proper Server-Side Apply behavior (atomic, set, or map)", fieldName),
+		})
+
+		return
+	}
+
+	for _, marker := range listTypeMarkers {
+		listType := marker.Expressions[""]
+
+		a.checkListTypeMarker(pass, listType, field)
+
+		if listType == listTypeMap {
+			a.checkListTypeMap(pass, fieldMarkers, field)
+		}
+
+		if listType == listTypeSet {
+			a.checkListTypeSet(pass, field)
+		}
+	}
+}
+
+func (a *analyzer) checkListTypeMarker(pass *analysis.Pass, listType string, field *ast.Field) {
+	fieldName := utils.FieldName(field)
+
+	if !validListType(listType) {
+		pass.Report(analysis.Diagnostic{
+			Pos:     field.Pos(),
+			Message: fmt.Sprintf("%s has invalid listType %q, must be one of: atomic, set, map", fieldName, listType),
+		})
+
+		return
+	}
+}
+
+func (a *analyzer) checkListTypeMap(pass *analysis.Pass, fieldMarkers markers.MarkerSet, field *ast.Field) {
+	listMapKeyMarkers := fieldMarkers.Get(kubebuildermarkers.KubebuilderListMapKeyMarker)
+	fieldName := utils.FieldName(field)
+
+	isObjectList := utils.IsObjectList(pass, field)
+
+	if !isObjectList {
+		pass.Report(analysis.Diagnostic{
+			Pos:     field.Pos(),
+			Message: fmt.Sprintf("%s with listType=map can only be used for object lists, not primitive lists", fieldName),
+		})
+
+		return
+	}
+
+	if len(listMapKeyMarkers) == 0 {
+		pass.Report(analysis.Diagnostic{
+			Pos:     field.Pos(),
+			Message: fmt.Sprintf("%s with listType=map must have at least one listMapKey marker", fieldName),
+		})
+
+		return
+	}
+
+	a.validateListMapKeys(pass, field, listMapKeyMarkers)
+}
+
+func (a *analyzer) checkListTypeSet(pass *analysis.Pass, field *ast.Field) {
+	if a.cfg.ListTypeSetUsage == config.SSATagsListTypeSetUsageIgnore {
+		return
+	}
+
+	isObjectList := utils.IsObjectList(pass, field)
+	if !isObjectList {
+		return
+	}
+
+	fieldName := utils.FieldName(field)
+	diagnostic := analysis.Diagnostic{
+		Pos:     field.Pos(),
+		Message: fmt.Sprintf("%s with listType=set is not recommended due to Server-Side Apply compatibility issues. Consider using listType=%s or listType=%s instead", fieldName, listTypeAtomic, listTypeMap),
+	}
+
+	pass.Report(diagnostic)
+}
+
+func (a *analyzer) validateListMapKeys(pass *analysis.Pass, field *ast.Field, listMapKeyMarkers []markers.Marker) {
+	jsonTags, ok := pass.ResultOf[extractjsontags.Analyzer].(extractjsontags.StructFieldTags)
+	if !ok {
+		return
+	}
+
+	structFields := a.getStructFieldsFromField(pass, field)
+	if structFields == nil {
+		return
+	}
+
+	fieldName := utils.FieldName(field)
+
+	for _, marker := range listMapKeyMarkers {
+		keyName := marker.Expressions[""]
+		if keyName == "" {
+			continue
+		}
+
+		if !a.hasFieldWithJSONTag(structFields, jsonTags, keyName) {
+			pass.Report(analysis.Diagnostic{
+				Pos:     field.Pos(),
+				Message: fmt.Sprintf("%s listMapKey %q does not exist as a field in the struct", fieldName, keyName),
+			})
+		}
+	}
+}
+
+func (a *analyzer) getStructFieldsFromField(pass *analysis.Pass, field *ast.Field) *ast.FieldList {
+	var elementType ast.Expr
+
+	// Get the element type from array or field type
+	if arrayType, ok := field.Type.(*ast.ArrayType); ok {
+		elementType = arrayType.Elt
+	} else {
+		elementType = field.Type
+	}
+
+	return a.getStructFieldsFromExpr(pass, elementType)
+}
+
+func (a *analyzer) getStructFieldsFromExpr(pass *analysis.Pass, expr ast.Expr) *ast.FieldList {
+	switch elementType := expr.(type) {
+	case *ast.Ident:
+		typeSpec, ok := utils.LookupTypeSpec(pass, elementType)
+		if !ok {
+			return nil
+		}
+
+		structType, ok := typeSpec.Type.(*ast.StructType)
+		if !ok {
+			return nil
+		}
+
+		return structType.Fields
+	case *ast.StarExpr:
+		return a.getStructFieldsFromExpr(pass, elementType.X)
+	case *ast.SelectorExpr:
+		return nil
+	}
+
+	return nil
+}
+
+func (a *analyzer) hasFieldWithJSONTag(fields *ast.FieldList, jsonTags extractjsontags.StructFieldTags, fieldName string) bool {
+	if fields == nil {
+		return false
+	}
+
+	for _, field := range fields.List {
+		tagInfo := jsonTags.FieldTags(field)
+
+		if tagInfo.Name == fieldName {
+			return true
+		}
+	}
+
+	return false
+}
+
+func validListType(listType string) bool {
+	switch listType {
+	case listTypeAtomic, listTypeSet, listTypeMap:
+		return true
+	default:
+		return false
+	}
+}
+
+func defaultConfig(cfg *config.SSATagsConfig) {
+	if cfg.ListTypeSetUsage == "" {
+		cfg.ListTypeSetUsage = config.SSATagsListTypeSetUsageWarn
+	}
+}