⚠️ Fix required fields linter findings (#12558)

* Add omitempty/omitzero to required fields

* Change Arg.Value from string to *string

* Change Required fields in ClusterClass struct from bool to *bool

* Change ResourceBinding.Applied from bool to *bool

* Change Partition.Layout from bool to *bool

Signed-off-by: Stefan Büringer [email protected]

* Change LastRemediationStatus.RetryCount from int32 to *int32

* Change UnhealthyNodeConditions TimeoutSeconds from int32 to *int32

* Change IPAddressSpec.Prefix from int to *int32

Signed-off-by: Stefan Büringer [email protected]

* Adjust KAL config to exclude requiredfields false positives

* Fix compile issue after rebase

---------

Signed-off-by: Stefan Büringer [email protected]

Author: sbueringer

.golangci-kal.yml

@@ -95,6 +95,10 @@ linters:
       text: "field Conditions type Conditions must have a maximum items, add kubebuilder:validation:MaxItems marker"
       linters:
         - kubeapilinter
+    - path: "api/core/v1beta2/condition_types.go"
+      text: "requiredfields: field (Type|Status|LastTransitionTime) should have the omitempty tag"
+      linters:
+        - kubeapilinter
 
     ## Excludes for current clusterctl v1alpha3 and Runtime Hooks v1alpha1 apiVersions (can be fixed once we bump their apiVersion).
     # Note: The types in api/runtime/hooks/v1alpha1 are not CRDs, so e.g. SSA markers don't make sense there.
@@ -208,6 +212,10 @@ linters:
       text: "requiredfields: field .* is marked as required, but has the omitempty tag"
       linters:
         - kubeapilinter
+    - path: "api/.*"
+      text: "requiredfields: field (Applied|Value|Layout|RetryCount|Required|Prefix|TimeoutSeconds) is marked as required, should not be a pointer"
+      linters:
+        - kubeapilinter
 
     # TODO: Excludes that should be removed once https://github.com/kubernetes-sigs/kube-api-linter/issues/132 will be fixed
     - path: "api/.*"
@@ -218,6 +226,7 @@ linters:
       text: "optionalfields: field (.*) is optional and has a valid zero value \\({}\\), but the validation is not complete \\(e.g. min properties/adding required fields\\). The field should be a pointer to allow the zero value to be set. If the zero value is not a valid use case, complete the validation and remove the pointer."
       linters:
         - kubeapilinter
+
 issues:
   max-same-issues: 0
   max-issues-per-linter: 0

api/addons/v1beta1/conversion_test.go

@@ -25,6 +25,7 @@ import (
 	"k8s.io/apimachinery/pkg/api/apitesting/fuzzer"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	runtimeserializer "k8s.io/apimachinery/pkg/runtime/serializer"
+	"k8s.io/utils/ptr"
 	"sigs.k8s.io/randfill"
 
 	addonsv1 "sigs.k8s.io/cluster-api/api/addons/v1beta2"
@@ -75,11 +76,20 @@ func spokeClusterResourceSetStatus(in *ClusterResourceSetStatus, c randfill.Cont
 
 func ClusterResourceSetBindingFuzzFuncs(_ runtimeserializer.CodecFactory) []interface{} {
 	return []interface{}{
+		hubResourceBinding,
 		hubClusterResourceSetStatus,
 		spokeClusterResourceSetBindingSpec,
 	}
 }
 
+func hubResourceBinding(in *addonsv1.ResourceBinding, c randfill.Continue) {
+	c.FillNoCustom(in)
+
+	if in.Applied == nil {
+		in.Applied = ptr.To(false) // Applied is a required field and nil does not round trip
+	}
+}
+
 func spokeClusterResourceSetBindingSpec(in *ClusterResourceSetBindingSpec, c randfill.Continue) {
 	c.FillNoCustom(in)
 

api/addons/v1beta1/zz_generated.conversion.go

@@ -58,14 +58,14 @@ type ClusterResourceSetSpec struct {
 	// It must match the Cluster labels. This field is immutable.
 	// Label selector cannot be empty.
 	// +required
-	ClusterSelector metav1.LabelSelector `json:"clusterSelector"`
+	ClusterSelector metav1.LabelSelector `json:"clusterSelector,omitempty,omitzero"`
 
 	// resources is a list of Secrets/ConfigMaps where each contains 1 or more resources to be applied to remote clusters.
 	// +required
 	// +listType=atomic
 	// +kubebuilder:validation:MinItems=1
 	// +kubebuilder:validation:MaxItems=100
-	Resources []ResourceRef `json:"resources"`
+	Resources []ResourceRef `json:"resources,omitempty"`
 
 	// strategy is the strategy to be used during applying resources. Defaults to ApplyOnce. This field is immutable.
 	// +kubebuilder:validation:Enum=ApplyOnce;Reconcile

api/addons/v1beta2/clusterresourceset_types.go

@@ -20,6 +20,7 @@ import (
 	"reflect"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/utils/ptr"
 )
 
 // ResourceBinding shows the status of a resource that belongs to a ClusterResourceSet matched by the owner cluster of the ClusterResourceSetBinding object.
@@ -40,7 +41,7 @@ type ResourceBinding struct {
 
 	// applied is to track if a resource is applied to the cluster or not.
 	// +required
-	Applied bool `json:"applied"`
+	Applied *bool `json:"applied,omitempty"`
 }
 
 // ResourceSetBinding keeps info on all of the resources in a ClusterResourceSet.
@@ -61,7 +62,7 @@ type ResourceSetBinding struct {
 // IsApplied returns true if the resource is applied to the cluster by checking the cluster's binding.
 func (r *ResourceSetBinding) IsApplied(resourceRef ResourceRef) bool {
 	resourceBinding := r.GetResource(resourceRef)
-	return resourceBinding != nil && resourceBinding.Applied
+	return resourceBinding != nil && ptr.Deref(resourceBinding.Applied, false)
 }
 
 // GetResource returns a ResourceBinding for a resource ref if present.

api/addons/v1beta2/clusterresourcesetbinding_types.go

@@ -23,6 +23,7 @@ import (
 	"github.com/google/go-cmp/cmp"
 	. "github.com/onsi/gomega"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/utils/ptr"
 )
 
 func TestIsResourceApplied(t *testing.T) {
@@ -43,13 +44,13 @@ func TestIsResourceApplied(t *testing.T) {
 		Resources: []ResourceBinding{
 			{
 				ResourceRef:     resourceRefApplySucceeded,
-				Applied:         true,
+				Applied:         ptr.To(true),
 				Hash:            "xyz",
 				LastAppliedTime: metav1.Time{Time: time.Now().UTC()},
 			},
 			{
 				ResourceRef:     resourceRefApplyFailed,
-				Applied:         false,
+				Applied:         ptr.To(false),
 				Hash:            "",
 				LastAppliedTime: metav1.Time{Time: time.Now().UTC()},
 			},
@@ -106,7 +107,7 @@ func TestResourceSetBindingGetResourceBinding(t *testing.T) {
 
 	resourceRefApplyFailedBinding := ResourceBinding{
 		ResourceRef:     resourceRefApplyFailed,
-		Applied:         false,
+		Applied:         ptr.To(false),
 		Hash:            "",
 		LastAppliedTime: metav1.Time{Time: time.Now().UTC()},
 	}
@@ -115,7 +116,7 @@ func TestResourceSetBindingGetResourceBinding(t *testing.T) {
 		Resources: []ResourceBinding{
 			{
 				ResourceRef:     resourceRefApplySucceeded,
-				Applied:         true,
+				Applied:         ptr.To(true),
 				Hash:            "xyz",
 				LastAppliedTime: metav1.Time{Time: time.Now().UTC()},
 			},
@@ -161,15 +162,15 @@ func TestSetResourceBinding(t *testing.T) {
 		Resources: []ResourceBinding{
 			{
 				ResourceRef:     resourceRefApplyFailed,
-				Applied:         false,
+				Applied:         ptr.To(false),
 				Hash:            "",
 				LastAppliedTime: metav1.Time{Time: time.Now().UTC()},
 			},
 		},
 	}
 	updateFailedResourceBinding := ResourceBinding{
 		ResourceRef:     resourceRefApplyFailed,
-		Applied:         true,
+		Applied:         ptr.To(true),
 		Hash:            "xyz",
 		LastAppliedTime: metav1.Time{Time: time.Now().UTC()},
 	}
@@ -179,7 +180,7 @@ func TestSetResourceBinding(t *testing.T) {
 			Name: "newBinding",
 			Kind: "Secret",
 		},
-		Applied:         false,
+		Applied:         ptr.To(false),
 		Hash:            "xyz",
 		LastAppliedTime: metav1.Time{Time: time.Now().UTC()},
 	}

api/addons/v1beta2/clusterresourcesetbinding_types_test.go

@@ -124,6 +124,50 @@ func hubKubeadmConfigSpec(in *bootstrapv1.KubeadmConfigSpec, c randfill.Continue
 	if in.ClusterConfiguration.Etcd.Local.ExtraEnvs != nil && *in.ClusterConfiguration.Etcd.Local.ExtraEnvs == nil {
 		in.ClusterConfiguration.Etcd.Local.ExtraEnvs = nil
 	}
+
+	for i, arg := range in.ClusterConfiguration.APIServer.ExtraArgs {
+		if arg.Value == nil {
+			arg.Value = ptr.To("")
+		}
+		in.ClusterConfiguration.APIServer.ExtraArgs[i] = arg
+	}
+	for i, arg := range in.ClusterConfiguration.ControllerManager.ExtraArgs {
+		if arg.Value == nil {
+			arg.Value = ptr.To("")
+		}
+		in.ClusterConfiguration.ControllerManager.ExtraArgs[i] = arg
+	}
+	for i, arg := range in.ClusterConfiguration.Scheduler.ExtraArgs {
+		if arg.Value == nil {
+			arg.Value = ptr.To("")
+		}
+		in.ClusterConfiguration.Scheduler.ExtraArgs[i] = arg
+	}
+	for i, arg := range in.ClusterConfiguration.Etcd.Local.ExtraArgs {
+		if arg.Value == nil {
+			arg.Value = ptr.To("")
+		}
+		in.ClusterConfiguration.Etcd.Local.ExtraArgs[i] = arg
+	}
+	for i, arg := range in.InitConfiguration.NodeRegistration.KubeletExtraArgs {
+		if arg.Value == nil {
+			arg.Value = ptr.To("")
+		}
+		in.InitConfiguration.NodeRegistration.KubeletExtraArgs[i] = arg
+	}
+	for i, arg := range in.JoinConfiguration.NodeRegistration.KubeletExtraArgs {
+		if arg.Value == nil {
+			arg.Value = ptr.To("")
+		}
+		in.JoinConfiguration.NodeRegistration.KubeletExtraArgs[i] = arg
+	}
+
+	for i, p := range in.DiskSetup.Partitions {
+		if p.Layout == nil {
+			p.Layout = ptr.To(false) // Layout is a required field and nil does not round trip
+		}
+		in.DiskSetup.Partitions[i] = p
+	}
 }
 
 func hubNodeRegistrationOptions(in *bootstrapv1.NodeRegistrationOptions, c randfill.Continue) {

api/addons/v1beta2/zz_generated.deepcopy.go

@@ -18,6 +18,8 @@ package v1beta2
 
 import (
 	"sort"
+
+	"k8s.io/utils/ptr"
 )
 
 func (*KubeadmConfig) Hub()         {}
@@ -36,11 +38,11 @@ func ConvertToArgs(in map[string]string) []Arg {
 	}
 	args := make([]Arg, 0, len(in))
 	for k, v := range in {
-		args = append(args, Arg{Name: k, Value: v})
+		args = append(args, Arg{Name: k, Value: ptr.To(v)})
 	}
 	sort.Slice(args, func(i, j int) bool {
 		if args[i].Name == args[j].Name {
-			return args[i].Value < args[j].Value
+			return ptr.Deref(args[i].Value, "") < ptr.Deref(args[j].Value, "")
 		}
 		return args[i].Name < args[j].Name
 	})
@@ -56,7 +58,7 @@ func ConvertFromArgs(in []Arg) map[string]string {
 	}
 	args := make(map[string]string, len(in))
 	for _, arg := range in {
-		args[arg.Name] = arg.Value
+		args[arg.Name] = ptr.Deref(arg.Value, "")
 	}
 	return args
 }