⚠️ Enable nomaps linter, Remove unused kubeadm ClusterStatus struct, Migrate Cluster.status.failureDomains to array (#12083)

* Add nomaps API linter and corresponding excludes

Signed-off-by: Stefan Büringer buringerst@vmware.com

* Remove unused bootstrap/kubeadm v1beta2 ClusterStatus struct

* Migrate Cluster.status.failureDomains from map to array

* Fix review findings

---------

Signed-off-by: Stefan Büringer buringerst@vmware.com

Author: sbueringer

.golangci-kal.yml

@@ -22,6 +22,7 @@ linters:
               - "maxlength" # Ensure all strings and arrays have maximum lengths/maximum items.
               - "nobools" # Bools do not evolve over time, should use enums instead.
               - "nofloats" # Ensure floats are not used.
+              - "nomaps" # Ensure maps are not used.
               - "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.
@@ -86,6 +87,14 @@ linters:
       text: "field Addresses type MachineAddresses must have a maximum items, add kubebuilder:validation:MaxItems marker"
       linters:
         - kubeapilinter
+    - path: "api/bootstrap/kubeadm/v1beta1/*"
+      text: "nomaps: APIEndpoints should not use a map type, use a list type with a unique name/identifier instead"
+      linters:
+        - kubeapilinter
+    - path: "api/core/v1beta1/*"
+      text: "nomaps: FailureDomains should not use a map type, use a list type with a unique name/identifier instead"
+      linters:
+        - kubeapilinter
 
     ## Excludes for clusterctl and Runtime Hooks (can be fixed once we bump their apiVersion)
     - path: "cmd/clusterctl/api/v1alpha3|api/runtime/hooks/v1alpha1"
@@ -111,6 +120,18 @@ linters:
       text: "field (XPreserveUnknownFields|XPreserveUnknownFields|XValidations|XMetadata|XIntOrString) json tag does not match pattern"
       linters:
         - kubeapilinter
+    # We want to align Properties to the corresponding field in CustomResourceDefinitions.
+    - path: "api/core/v1beta2/*|api/core/v1beta1/*"
+      text: "Properties should not use a map type, use a list type with a unique name/identifier instead"
+      linters:
+        - kubeapilinter
+
+    ## Excludes for kubeadm types
+    # We want to align the FeatureGates field to the FeatureGates field in kubeadm.
+    - path: "api/bootstrap/kubeadm/v1beta2/*|api/bootstrap/kubeadm/v1beta1/*"
+      text: "nomaps: FeatureGates should not use a map type, use a list type with a unique name/identifier instead"
+      linters:
+        - kubeapilinter
 
     ## TODO: The following rules are disabled until we migrate to the new API.
     # Note: Maybe this has to stay a pointer for marshalling reasons.

api/bootstrap/kubeadm/v1beta1/zz_generated.conversion.go

@@ -29,7 +29,6 @@ func (*KubeadmConfig) Hub()         {}
 func (*KubeadmConfigTemplate) Hub() {}
 
 func (*ClusterConfiguration) Hub() {}
-func (*ClusterStatus) Hub()        {}
 func (*InitConfiguration) Hub()    {}
 func (*JoinConfiguration) Hub()    {}
 

api/bootstrap/kubeadm/v1beta2/conversion.go

@@ -239,22 +239,6 @@ type ImageMeta struct {
 	// TODO: evaluate if we need also a ImageName based on user feedbacks
 }
 
-// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
-
-// ClusterStatus contains the cluster status. The ClusterStatus will be stored in the kubeadm-config
-// ConfigMap in the cluster, and then updated by kubeadm when additional control plane instance joins or leaves the cluster.
-//
-// Deprecated: ClusterStatus has been removed from kubeadm v1beta3 API; This type is preserved only to support
-// conversion to older versions of the kubeadm API.
-type ClusterStatus struct {
-	metav1.TypeMeta `json:",inline"`
-
-	// apiEndpoints currently available in the cluster, one for each control plane/api server instance.
-	// The key of the map is the IP of the host's default interface
-	// +optional
-	APIEndpoints map[string]APIEndpoint `json:"apiEndpoints"`
-}
-
 // APIEndpoint struct contains elements of API server instance deployed on a node.
 type APIEndpoint struct {
 	// advertiseAddress sets the IP address for the API server to advertise.

api/bootstrap/kubeadm/v1beta2/kubeadm_types.go

@@ -321,6 +321,17 @@ func Convert_v1beta2_ClusterStatus_To_v1beta1_ClusterStatus(in *clusterv1.Cluste
 		out.InfrastructureReady = in.Initialization.InfrastructureProvisioned
 	}
 
+	// Move FailureDomains
+	if in.FailureDomains != nil {
+		out.FailureDomains = FailureDomains{}
+		for _, fd := range in.FailureDomains {
+			out.FailureDomains[fd.Name] = FailureDomainSpec{
+				ControlPlane: fd.ControlPlane,
+				Attributes:   fd.Attributes,
+			}
+		}
+	}
+
 	// Move new conditions (v1beta2), controlPlane and workers counters to the v1beta2 field.
 	if in.Conditions == nil && in.ControlPlane == nil && in.Workers == nil {
 		return nil
@@ -399,6 +410,18 @@ func Convert_v1beta1_ClusterStatus_To_v1beta2_ClusterStatus(in *ClusterStatus, o
 		out.Initialization.InfrastructureProvisioned = in.InfrastructureReady
 	}
 
+	// Move FailureDomains
+	if in.FailureDomains != nil {
+		out.FailureDomains = []clusterv1.FailureDomain{}
+		for name, fd := range in.FailureDomains {
+			out.FailureDomains = append(out.FailureDomains, clusterv1.FailureDomain{
+				Name:         name,
+				ControlPlane: fd.ControlPlane,
+				Attributes:   fd.Attributes,
+			})
+		}
+	}
+
 	// Move legacy conditions (v1beta1), FailureReason and FailureMessage to the deprecated field.
 	if in.Conditions == nil && in.FailureReason == nil && in.FailureMessage == nil {
 		return nil

api/bootstrap/kubeadm/v1beta2/zz_generated.deepcopy.go

@@ -26,7 +26,6 @@ import (
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
-	"k8s.io/utils/ptr"
 
 	capierrors "sigs.k8s.io/cluster-api/errors"
 )
@@ -976,7 +975,11 @@ type ClusterStatus struct {
 
 	// failureDomains is a slice of failure domain objects synced from the infrastructure provider.
 	// +optional
-	FailureDomains FailureDomains `json:"failureDomains,omitempty"`
+	// +listType=map
+	// +listMapKey=name
+	// +kubebuilder:validation:MinItems=1
+	// +kubebuilder:validation:MaxItems=100
+	FailureDomains []FailureDomain `json:"failureDomains,omitempty"`
 
 	// phase represents the current phase of cluster actuation.
 	// +optional
@@ -1235,33 +1238,15 @@ func init() {
 	objectTypes = append(objectTypes, &Cluster{}, &ClusterList{})
 }
 
-// FailureDomains is a slice of FailureDomains.
-type FailureDomains map[string]FailureDomainSpec
-
-// FilterControlPlane returns a FailureDomain slice containing only the domains suitable to be used
-// for control plane nodes.
-func (in FailureDomains) FilterControlPlane() FailureDomains {
-	res := make(FailureDomains)
-	for id, spec := range in {
-		if spec.ControlPlane {
-			res[id] = spec
-		}
-	}
-	return res
-}
-
-// GetIDs returns a slice containing the ids for failure domains.
-func (in FailureDomains) GetIDs() []*string {
-	ids := make([]*string, 0, len(in))
-	for id := range in {
-		ids = append(ids, ptr.To(id))
-	}
-	return ids
-}
-
-// FailureDomainSpec is the Schema for Cluster API failure domains.
+// FailureDomain is the Schema for Cluster API failure domains.
 // It allows controllers to understand how many failure domains a cluster can optionally span across.
-type FailureDomainSpec struct {
+type FailureDomain struct {
+	// name is the name of the failure domain.
+	// +required
+	// +kubebuilder:validation:MinLength=1
+	// +kubebuilder:validation:MaxLength=256
+	Name string `json:"name"`
+
 	// controlPlane determines if this failure domain is suitable for use by control plane machines.
 	// +optional
 	ControlPlane bool `json:"controlPlane,omitempty"`

api/core/v1beta1/conversion.go

@@ -280,7 +280,7 @@ type MachineDeploymentClass struct {
 	MachineHealthCheck *MachineHealthCheckClass `json:"machineHealthCheck,omitempty"`
 
 	// failureDomain is the failure domain the machines will be created in.
-	// Must match a key in the FailureDomains map stored on the cluster object.
+	// Must match the name of a FailureDomain from the Cluster status.
 	// NOTE: This value can be overridden while defining a Cluster.Topology using this MachineDeploymentClass.
 	// +optional
 	// +kubebuilder:validation:MinLength=1

api/core/v1beta1/zz_generated.conversion.go

@@ -418,7 +418,7 @@ type MachineSpec struct {
 	ProviderID *string `json:"providerID,omitempty"`
 
 	// failureDomain is the failure domain the machine will be created in.
-	// Must match a key in the FailureDomains map stored on the cluster object.
+	// Must match the name of a FailureDomain from the Cluster status.
 	// +optional
 	// +kubebuilder:validation:MinLength=1
 	// +kubebuilder:validation:MaxLength=256