Enable optionalfields linter

Author: sbueringer

.golangci-kal.yml

@@ -23,6 +23,8 @@ linters:
               - "nobools" # Bools do not evolve over time, should use enums instead.
               - "nofloats" # Ensure floats are not used.
               - "nomaps" # Ensure maps are not used.
+              - "optionalfields" # Ensure that all fields marked as optional adhere to being pointers and
+                                 # having the `omitempty` value in their `json` tag where appropriate.
               - "optionalorrequired" # Every field should be marked as `+optional` or `+required`.
               - "requiredfields" # Required fields should not be pointers, and should not have `omitempty`.
               - "statusoptional" # Ensure all first children within status should be optional.
@@ -42,6 +44,12 @@ linters:
               isFirstField: Warn # Require conditions to be the first field in the status struct.
               usePatchStrategy: Forbid # Require conditions to be the first field in the status struct.
               useProtobuf: Forbid # We don't use protobuf, so protobuf tags are not required.
+            optionalFields:
+              pointers:
+                preference: WhenRequired # Always | WhenRequired # Whether to always require pointers, or only when required. Defaults to `Always`.
+                policy: SuggestFix # SuggestFix | Warn # The policy for pointers in optional fields. Defaults to `SuggestFix`.
+              omitempty:
+                policy: SuggestFix # SuggestFix | Warn | Ignore # The policy for omitempty in optional fields. Defaults to `SuggestFix`.
           # jsonTags:
           #   jsonTagRegex: "^[a-z][a-z0-9]*(?:[A-Z][a-z0-9]*)*$" # The default regex is appropriate for our use case.
           # optionalOrRequired:
@@ -71,6 +79,10 @@ linters:
       text: "Conditions field must be a slice of metav1.Condition"
       linters:
         - kubeapilinter
+    - path: "api/addons/v1beta1/*|api/bootstrap/kubeadm/v1beta1/*|api/controlplane/kubeadm/v1beta1/*|api/core/v1beta1/*|api/ipam/v1beta1/*|api/ipam/v1alpha1/*|api/runtime/v1alpha1/*|cmd/clusterctl/api/v1alpha3/*"
+      text: "optionalfields"
+      linters:
+        - kubeapilinter
     - path: "api/core/v1beta2/*|api/core/v1beta1/*"
       text: "field Conditions type Conditions must have a maximum items, add kubebuilder:validation:MaxItems marker"
       linters:
@@ -144,6 +156,30 @@ linters:
       text: "field Ref is marked as required, should not be a pointer"
       linters:
         - kubeapilinter
+
+    ## TODO: optional required tasks.
+    # Move up
+    # Audit the entire hook types + builtins from a serialization point of view (this is not a CRD)
+    - path: "api/runtime/hooks/v1alpha1/*"
+      text: "optionalfields"
+      linters:
+        - kubeapilinter
+
+    # https://github.com/kubernetes-sigs/cluster-api/pull/12429
+    # FailureMessage will be removed when v1beta1 is removed
+    - path: "api/*"
+      text: "field FailureMessage is optional and does not allow the zero value"
+      linters:
+        - kubeapilinter
+
+    # https://github.com/kubernetes-sigs/cluster-api/pull/12431
+    # https://github.com/kubernetes-sigs/cluster-api/pull/12435
+    # Take a closer look
+    - path: ".*"
+      text: "field Spec|Status is optional"
+      linters:
+        - kubeapilinter
+
 issues:
   max-same-issues: 0
   max-issues-per-linter: 0

api/bootstrap/kubeadm/v1beta2/kubeadm_types.go

@@ -85,7 +85,8 @@ type InitConfiguration struct {
 	// When used in the context of control plane nodes, NodeRegistration should remain consistent
 	// across both InitConfiguration and JoinConfiguration
 	// +optional
-	NodeRegistration NodeRegistrationOptions `json:"nodeRegistration,omitempty"`
+	// +kubebuilder:validation:MinProperties=1
+	NodeRegistration NodeRegistrationOptions `json:"nodeRegistration,omitempty,omitzero"` // nolint:kubeapilinter // nolint:kubeapilinter
 
 	// localAPIEndpoint represents the endpoint of the API server instance that's deployed on this control plane node
 	// In HA setups, this differs from ClusterConfiguration.ControlPlaneEndpoint in the sense that ControlPlaneEndpoint
@@ -94,7 +95,8 @@ type InitConfiguration struct {
 	// on. By default, kubeadm tries to auto-detect the IP of the default interface and use that, but in case that process
 	// fails you may set the desired value here.
 	// +optional
-	LocalAPIEndpoint APIEndpoint `json:"localAPIEndpoint,omitempty"`
+	// +kubebuilder:validation:MinProperties=1
+	LocalAPIEndpoint APIEndpoint `json:"localAPIEndpoint,omitempty,omitzero"` // nolint:kubeapilinter
 
 	// skipPhases is a list of phases to skip during command execution.
 	// The list of phases can be obtained with the "kubeadm init --help" command.
@@ -120,7 +122,8 @@ type ClusterConfiguration struct {
 	// etcd holds configuration for etcd.
 	// NB: This value defaults to a Local (stacked) etcd
 	// +optional
-	Etcd Etcd `json:"etcd,omitempty"`
+	// +kubebuilder:validation:MinProperties=1
+	Etcd Etcd `json:"etcd,omitempty,omitzero"` // nolint:kubeapilinter // FIXME: double check that unsetting etcd still works: https://github.com/kubernetes-sigs/cluster-api/pull/12065
 
 	// controlPlaneEndpoint sets a stable IP address or DNS name for the control plane; it
 	// can be a valid IP address or a RFC-1123 DNS subdomain, both with optional TCP port.
@@ -141,19 +144,23 @@ type ClusterConfiguration struct {
 
 	// apiServer contains extra settings for the API server control plane component
 	// +optional
-	APIServer APIServer `json:"apiServer,omitempty"`
+	// +kubebuilder:validation:MinProperties=1
+	APIServer APIServer `json:"apiServer,omitempty,omitzero"` // nolint:kubeapilinter
 
 	// controllerManager contains extra settings for the controller manager control plane component
 	// +optional
-	ControllerManager ControlPlaneComponent `json:"controllerManager,omitempty"`
+	// +kubebuilder:validation:MinProperties=1
+	ControllerManager ControlPlaneComponent `json:"controllerManager,omitempty,omitzero"` // nolint:kubeapilinter // FIXME: replace with ControllerManager struct
 
 	// scheduler contains extra settings for the scheduler control plane component
 	// +optional
-	Scheduler ControlPlaneComponent `json:"scheduler,omitempty"`
+	// +kubebuilder:validation:MinProperties=1
+	Scheduler ControlPlaneComponent `json:"scheduler,omitempty,omitzero"` // nolint:kubeapilinter // FIXME: replace with Scheduler struct
 
 	// dns defines the options for the DNS add-on installed in the cluster.
 	// +optional
-	DNS DNS `json:"dns,omitempty"`
+	// +kubebuilder:validation:MinProperties=1
+	DNS DNS `json:"dns,omitempty,omitzero"` // nolint:kubeapilinter
 
 	// certificatesDir specifies where to store or look for all required certificates.
 	// NB: if not provided, this will default to `/etc/kubernetes/pki`
@@ -511,7 +518,8 @@ type JoinConfiguration struct {
 	// When used in the context of control plane nodes, NodeRegistration should remain consistent
 	// across both InitConfiguration and JoinConfiguration
 	// +optional
-	NodeRegistration NodeRegistrationOptions `json:"nodeRegistration,omitempty"`
+	// +kubebuilder:validation:MinProperties=1
+	NodeRegistration NodeRegistrationOptions `json:"nodeRegistration,omitempty,omitzero"` // nolint:kubeapilinter
 
 	// caCertPath is the path to the SSL certificate authority used to
 	// secure communications between node and control-plane.
@@ -525,7 +533,9 @@ type JoinConfiguration struct {
 	// discovery specifies the options for the kubelet to use during the TLS Bootstrap process
 	// +optional
 	// TODO: revisit when there is defaulting from k/k
-	Discovery Discovery `json:"discovery,omitempty"`
+	// +kubebuilder:validation:MinProperties=1
+	// FIXME: double check that one of BootstrapToken or File must be set
+	Discovery Discovery `json:"discovery,omitempty,omitzero"` // nolint:kubeapilinter
 
 	// controlPlane defines the additional control plane instance to be deployed on the joining node.
 	// If nil, no additional control plane instance will be deployed.
@@ -555,7 +565,8 @@ type JoinConfiguration struct {
 type JoinControlPlane struct {
 	// localAPIEndpoint represents the endpoint of the API server instance to be deployed on this node.
 	// +optional
-	LocalAPIEndpoint APIEndpoint `json:"localAPIEndpoint,omitempty"`
+	// +kubebuilder:validation:MinProperties=1
+	LocalAPIEndpoint APIEndpoint `json:"localAPIEndpoint,omitempty,omitzero"` // nolint:kubeapilinter
 }
 
 // Discovery specifies the options for the kubelet to use during the TLS Bootstrap process.

api/bootstrap/kubeadm/v1beta2/kubeadmconfig_types.go

@@ -116,7 +116,7 @@ type KubeadmConfigSpec struct {
 
 	// format specifies the output format of the bootstrap data
 	// +optional
-	Format Format `json:"format,omitempty"`
+	Format Format `json:"format,omitempty"` // nolint:kubeapilinter // FIXME: Talk to Joel: optionalfields linter should be improved to detect enum on the type
 
 	// verbosity is the number for the kubeadm log level verbosity.
 	// It overrides the `--v` flag in kubeadm commands.
@@ -630,7 +630,7 @@ type File struct {
 
 	// encoding specifies the encoding of the file contents.
 	// +optional
-	Encoding Encoding `json:"encoding,omitempty"`
+	Encoding Encoding `json:"encoding,omitempty"` // nolint:kubeapilinter // FIXME: Talk to Joel: optionalfields linter should be improved to detect enum on the type
 
 	// append specifies whether to append Content to existing file if Path exists.
 	// +optional

api/controlplane/kubeadm/v1beta2/kubeadm_control_plane_types.go

@@ -538,7 +538,7 @@ type RolloutStrategy struct {
 	// "RollingUpdate".
 	// Default is RollingUpdate.
 	// +optional
-	Type RolloutStrategyType `json:"type,omitempty"`
+	Type RolloutStrategyType `json:"type,omitempty"` // nolint:kubeapilinter // FIXME: Talk to Joel: optionalfields linter should be improved to detect enum on the type
 
 	// rollingUpdate is the rolling update config params. Present only if
 	// RolloutStrategyType = RollingUpdate.

api/core/v1beta2/cluster_types.go

@@ -549,7 +549,8 @@ type Topology struct {
 
 	// controlPlane describes the cluster control plane.
 	// +optional
-	ControlPlane ControlPlaneTopology `json:"controlPlane,omitempty"`
+	// +kubebuilder:validation:MinProperties=1
+	ControlPlane ControlPlaneTopology `json:"controlPlane,omitempty,omitzero"` // nolint:kubeapilinter
 
 	// workers encapsulates the different constructs that form the worker nodes
 	// for the cluster.
@@ -597,7 +598,8 @@ type ControlPlaneTopology struct {
 	// is applied only to the ControlPlane.
 	// At runtime this metadata is merged with the corresponding metadata from the ClusterClass.
 	// +optional
-	Metadata ObjectMeta `json:"metadata,omitempty"`
+	// +kubebuilder:validation:MinProperties=1
+	Metadata ObjectMeta `json:"metadata,omitempty,omitzero"` // nolint:kubeapilinter
 
 	// replicas is the number of control plane nodes.
 	// If the value is nil, the ControlPlane object is created without the number of Replicas
@@ -675,7 +677,8 @@ type MachineDeploymentTopology struct {
 	// metadata is the metadata applied to the MachineDeployment and the machines of the MachineDeployment.
 	// At runtime this metadata is merged with the corresponding metadata from the ClusterClass.
 	// +optional
-	Metadata ObjectMeta `json:"metadata,omitempty"`
+	// +kubebuilder:validation:MinProperties=1
+	Metadata ObjectMeta `json:"metadata,omitempty,omitzero"` // nolint:kubeapilinter
 
 	// class is the name of the MachineDeploymentClass used to create the set of worker nodes.
 	// This should match one of the deployment classes defined in the ClusterClass object
@@ -789,7 +792,8 @@ type MachinePoolTopology struct {
 	// metadata is the metadata applied to the MachinePool.
 	// At runtime this metadata is merged with the corresponding metadata from the ClusterClass.
 	// +optional
-	Metadata ObjectMeta `json:"metadata,omitempty"`
+	// +kubebuilder:validation:MinProperties=1
+	Metadata ObjectMeta `json:"metadata,omitempty,omitzero"` // nolint:kubeapilinter
 
 	// class is the name of the MachinePoolClass used to create the pool of worker nodes.
 	// This should match one of the deployment classes defined in the ClusterClass object

api/core/v1beta2/clusterclass_types.go

@@ -104,18 +104,21 @@ type ClusterClassSpec struct {
 	// infrastructure is a reference to a local struct that holds the details
 	// for provisioning the infrastructure cluster for the Cluster.
 	// +optional
-	Infrastructure InfrastructureClass `json:"infrastructure,omitempty"`
+	// +kubebuilder:validation:MinProperties=1
+	Infrastructure InfrastructureClass `json:"infrastructure,omitempty,omitzero"` // nolint:kubeapilinter
 
 	// controlPlane is a reference to a local struct that holds the details
 	// for provisioning the Control Plane for the Cluster.
 	// +optional
-	ControlPlane ControlPlaneClass `json:"controlPlane,omitempty"`
+	// +kubebuilder:validation:MinProperties=1
+	ControlPlane ControlPlaneClass `json:"controlPlane,omitempty,omitzero"` // nolint:kubeapilinter
 
 	// workers describes the worker nodes for the cluster.
 	// It is a collection of node types which can be used to create
 	// the worker nodes of the cluster.
 	// +optional
-	Workers WorkersClass `json:"workers,omitempty"`
+	// +kubebuilder:validation:MinProperties=1
+	Workers WorkersClass `json:"workers,omitempty,omitzero"` // nolint:kubeapilinter
 
 	// variables defines the variables which can be configured
 	// in the Cluster topology and are then used in patches.
@@ -151,7 +154,8 @@ type ControlPlaneClass struct {
 	// This field is supported if and only if the control plane provider template
 	// referenced is Machine based.
 	// +optional
-	Metadata ObjectMeta `json:"metadata,omitempty"`
+	// +kubebuilder:validation:MinProperties=1
+	Metadata ObjectMeta `json:"metadata,omitempty,omitzero"` // nolint:kubeapilinter
 
 	// LocalObjectTemplate contains the reference to a provider-specific control plane template.
 	ClusterClassTemplate `json:",inline"`
@@ -354,7 +358,8 @@ type MachineDeploymentClassTemplate struct {
 	// metadata is the metadata applied to the MachineDeployment and the machines of the MachineDeployment.
 	// At runtime this metadata is merged with the corresponding metadata from the topology.
 	// +optional
-	Metadata ObjectMeta `json:"metadata,omitempty"`
+	// +kubebuilder:validation:MinProperties=1
+	Metadata ObjectMeta `json:"metadata,omitempty,omitzero"` // nolint:kubeapilinter
 
 	// bootstrap contains the bootstrap template reference to be used
 	// for the creation of worker Machines.
@@ -504,7 +509,8 @@ type MachinePoolClassTemplate struct {
 	// metadata is the metadata applied to the MachinePool.
 	// At runtime this metadata is merged with the corresponding metadata from the topology.
 	// +optional
-	Metadata ObjectMeta `json:"metadata,omitempty"`
+	// +kubebuilder:validation:MinProperties=1
+	Metadata ObjectMeta `json:"metadata,omitempty,omitzero"` // nolint:kubeapilinter
 
 	// bootstrap contains the bootstrap template reference to be used
 	// for the creation of the Machines in the MachinePool.
@@ -561,7 +567,8 @@ type ClusterClassVariable struct {
 	// Deprecated: This field is deprecated and will be removed when support for v1beta1 will be dropped. Please use XMetadata in JSONSchemaProps instead.
 	//
 	// +optional
-	DeprecatedV1Beta1Metadata ClusterClassVariableMetadata `json:"deprecatedV1Beta1Metadata,omitempty"`
+	// +kubebuilder:validation:MinProperties=1
+	DeprecatedV1Beta1Metadata ClusterClassVariableMetadata `json:"deprecatedV1Beta1Metadata,omitempty,omitzero"` // nolint:kubeapilinter
 
 	// schema defines the schema of the variable.
 	// +required
@@ -1288,7 +1295,8 @@ type ClusterClassStatusVariableDefinition struct {
 	// Deprecated: This field is deprecated and will be removed when support for v1beta1 will be dropped. Please use XMetadata in JSONSchemaProps instead.
 	//
 	// +optional
-	DeprecatedV1Beta1Metadata ClusterClassVariableMetadata `json:"deprecatedV1Beta1Metadata,omitempty"`
+	// +kubebuilder:validation:MinProperties=1
+	DeprecatedV1Beta1Metadata ClusterClassVariableMetadata `json:"deprecatedV1Beta1Metadata,omitempty,omitzero"` // nolint:kubeapilinter
 
 	// schema defines the schema of the variable.
 	// +required

api/core/v1beta2/condition_types.go

@@ -70,7 +70,7 @@ type Condition struct {
 	// understand the current situation and act accordingly.
 	// The Severity field MUST be set only when Status=False.
 	// +optional
-	Severity ConditionSeverity `json:"severity,omitempty"`
+	Severity ConditionSeverity `json:"severity,omitempty"` // nolint:kubeapilinter // field will be removed when v1beta1 is removed
 
 	// lastTransitionTime is the last time the condition transitioned from one status to another.
 	// This should be when the underlying condition changed. If that is not known, then using the time when

api/ipam/v1beta2/ipaddressclaim_types.go

@@ -65,7 +65,7 @@ type IPAddressClaimStatus struct {
 
 	// addressRef is a reference to the address that was created for this claim.
 	// +optional
-	AddressRef IPAddressReference `json:"addressRef,omitempty"`
+	AddressRef IPAddressReference `json:"addressRef,omitempty,omitzero"` // nolint:kubeapilinter // FIXME: Talk to Joel: optionalfields linter should be improved to detect omitzero
 
 	// deprecated groups all the status fields that are deprecated and will be removed when all the nested field are removed.
 	// +optional

api/runtime/v1beta2/extensionconfig_types.go

@@ -179,7 +179,7 @@ type ExtensionHandler struct {
 	// Defaults to Fail if not set.
 	// +optional
 	// +kubebuilder:validation:Enum=Ignore;Fail
-	FailurePolicy *FailurePolicy `json:"failurePolicy,omitempty"`
+	FailurePolicy *FailurePolicy `json:"failurePolicy,omitempty"`  // nolint:kubeapilinter // FIXME: change this to non-pointer
 }
 
 // GroupVersionHook defines the runtime hook when the ExtensionHandler is called.