diff --git a/api/v1beta1/cluster_types.go b/api/v1beta1/cluster_types.go index 3f785e618435..848d6f289e35 100644 --- a/api/v1beta1/cluster_types.go +++ b/api/v1beta1/cluster_types.go @@ -153,10 +153,14 @@ type ControlPlaneTopology struct { type WorkersTopology struct { // MachineDeployments is a list of machine deployments in the cluster. // +optional + // +listType=map + // +listMapKey=name MachineDeployments []MachineDeploymentTopology `json:"machineDeployments,omitempty"` // MachinePools is a list of machine pools in the cluster. // +optional + // +listType=map + // +listMapKey=name MachinePools []MachinePoolTopology `json:"machinePools,omitempty"` } diff --git a/api/v1beta1/clusterclass_types.go b/api/v1beta1/clusterclass_types.go index 34c7ca7cf420..38c2530cbdd6 100644 --- a/api/v1beta1/clusterclass_types.go +++ b/api/v1beta1/clusterclass_types.go @@ -149,11 +149,15 @@ type WorkersClass struct { // MachineDeployments is a list of machine deployment classes that can be used to create // a set of worker nodes. // +optional + // +listType=map + // +listMapKey=class MachineDeployments []MachineDeploymentClass `json:"machineDeployments,omitempty"` // MachinePools is a list of machine pool classes that can be used to create // a set of worker nodes. // +optional + // +listType=map + // +listMapKey=class MachinePools []MachinePoolClass `json:"machinePools,omitempty"` } diff --git a/api/v1beta1/zz_generated.openapi.go b/api/v1beta1/zz_generated.openapi.go index d8bf4fdc7245..90b3ae2d14ad 100644 --- a/api/v1beta1/zz_generated.openapi.go +++ b/api/v1beta1/zz_generated.openapi.go @@ -3860,6 +3860,14 @@ func schema_sigsk8sio_cluster_api_api_v1beta1_WorkersClass(ref common.ReferenceC Type: []string{"object"}, Properties: map[string]spec.Schema{ "machineDeployments": { + VendorExtensible: spec.VendorExtensible{ + Extensions: spec.Extensions{ + "x-kubernetes-list-map-keys": []interface{}{ + "class", + }, + "x-kubernetes-list-type": "map", + }, + }, SchemaProps: spec.SchemaProps{ Description: "MachineDeployments is a list of machine deployment classes that can be used to create a set of worker nodes.", Type: []string{"array"}, @@ -3874,6 +3882,14 @@ func schema_sigsk8sio_cluster_api_api_v1beta1_WorkersClass(ref common.ReferenceC }, }, "machinePools": { + VendorExtensible: spec.VendorExtensible{ + Extensions: spec.Extensions{ + "x-kubernetes-list-map-keys": []interface{}{ + "class", + }, + "x-kubernetes-list-type": "map", + }, + }, SchemaProps: spec.SchemaProps{ Description: "MachinePools is a list of machine pool classes that can be used to create a set of worker nodes.", Type: []string{"array"}, @@ -3903,6 +3919,14 @@ func schema_sigsk8sio_cluster_api_api_v1beta1_WorkersTopology(ref common.Referen Type: []string{"object"}, Properties: map[string]spec.Schema{ "machineDeployments": { + VendorExtensible: spec.VendorExtensible{ + Extensions: spec.Extensions{ + "x-kubernetes-list-map-keys": []interface{}{ + "name", + }, + "x-kubernetes-list-type": "map", + }, + }, SchemaProps: spec.SchemaProps{ Description: "MachineDeployments is a list of machine deployments in the cluster.", Type: []string{"array"}, @@ -3917,6 +3941,14 @@ func schema_sigsk8sio_cluster_api_api_v1beta1_WorkersTopology(ref common.Referen }, }, "machinePools": { + VendorExtensible: spec.VendorExtensible{ + Extensions: spec.Extensions{ + "x-kubernetes-list-map-keys": []interface{}{ + "name", + }, + "x-kubernetes-list-type": "map", + }, + }, SchemaProps: spec.SchemaProps{ Description: "MachinePools is a list of machine pools in the cluster.", Type: []string{"array"}, diff --git a/config/crd/bases/cluster.x-k8s.io_clusterclasses.yaml b/config/crd/bases/cluster.x-k8s.io_clusterclasses.yaml index 225f5d9dc69c..ee9c23b262f1 100644 --- a/config/crd/bases/cluster.x-k8s.io_clusterclasses.yaml +++ b/config/crd/bases/cluster.x-k8s.io_clusterclasses.yaml @@ -1757,6 +1757,9 @@ spec: - template type: object type: array + x-kubernetes-list-map-keys: + - class + x-kubernetes-list-type: map machinePools: description: |- MachinePools is a list of machine pool classes that can be used to create @@ -1971,6 +1974,9 @@ spec: - template type: object type: array + x-kubernetes-list-map-keys: + - class + x-kubernetes-list-type: map type: object type: object status: diff --git a/config/crd/bases/cluster.x-k8s.io_clusters.yaml b/config/crd/bases/cluster.x-k8s.io_clusters.yaml index 0935c3f5ec15..acb5591aacc2 100644 --- a/config/crd/bases/cluster.x-k8s.io_clusters.yaml +++ b/config/crd/bases/cluster.x-k8s.io_clusters.yaml @@ -1540,6 +1540,9 @@ spec: - name type: object type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map machinePools: description: MachinePools is a list of machine pools in the cluster. @@ -1668,6 +1671,9 @@ spec: - name type: object type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map type: object required: - class diff --git a/internal/topology/variables/cluster_variable_validation_test.go b/internal/topology/variables/cluster_variable_validation_test.go index 8c24469feb8d..e83f5cc05f92 100644 --- a/internal/topology/variables/cluster_variable_validation_test.go +++ b/internal/topology/variables/cluster_variable_validation_test.go @@ -2354,7 +2354,7 @@ func Test_ValidateMachineVariables(t *testing.T) { name: "Error if value has no definition.", wantErrs: []validationMatch{ invalid("Invalid value: \"\\\"us-east-1\\\"\": no definitions found for variable \"location\"", - "spec.topology.workers.machineDeployments[0].variables.overrides[location]"), + "spec.topology.workers.machineDeployments[mdTopologyName].variables.overrides[location]"), }, definitions: []clusterv1.ClusterClassStatusVariable{}, values: []clusterv1.ClusterVariable{ @@ -2371,7 +2371,7 @@ func Test_ValidateMachineVariables(t *testing.T) { name: "Fail if value DefinitionFrom field does not match any definition.", wantErrs: []validationMatch{ invalid("Invalid value: \"1\": no definitions found for variable \"cpu\" from \"non-existent-patch\"", - "spec.topology.workers.machineDeployments[0].variables.overrides[cpu]"), + "spec.topology.workers.machineDeployments[mdTopologyName].variables.overrides[cpu]"), }, definitions: []clusterv1.ClusterClassStatusVariable{ { @@ -2403,9 +2403,9 @@ func Test_ValidateMachineVariables(t *testing.T) { name: "Fail when values invalid by their definition schema.", wantErrs: []validationMatch{ invalidType("Invalid value: \"1\": must be of type string: \"integer\"", - "spec.topology.workers.machineDeployments[0].variables.overrides[cpu].value"), + "spec.topology.workers.machineDeployments[mdTopologyName].variables.overrides[cpu].value"), invalidType("Invalid value: \"\\\"one\\\"\": must be of type integer: \"string\"", - "spec.topology.workers.machineDeployments[0].variables.overrides[cpu].value"), + "spec.topology.workers.machineDeployments[mdTopologyName].variables.overrides[cpu].value"), }, definitions: []clusterv1.ClusterClassStatusVariable{ { @@ -2483,7 +2483,7 @@ func Test_ValidateMachineVariables(t *testing.T) { name: "Error if integer is above maximum via with CEL expression", wantErrs: []validationMatch{ invalid("Invalid value: \"99\": failed rule: self <= 1", - "spec.topology.workers.machineDeployments[0].variables.overrides[cpu].value"), + "spec.topology.workers.machineDeployments[mdTopologyName].variables.overrides[cpu].value"), }, definitions: []clusterv1.ClusterClassStatusVariable{ { @@ -2517,7 +2517,7 @@ func Test_ValidateMachineVariables(t *testing.T) { name: "Error if integer is below minimum via CEL expression", wantErrs: []validationMatch{ invalid("Invalid value: \"0\": failed rule: self >= 1", - "spec.topology.workers.machineDeployments[0].variables.overrides[cpu].value"), + "spec.topology.workers.machineDeployments[mdTopologyName].variables.overrides[cpu].value"), }, definitions: []clusterv1.ClusterClassStatusVariable{ { @@ -2619,7 +2619,7 @@ func Test_ValidateMachineVariables(t *testing.T) { name: "Error if integer is not greater than old value via CEL expression", wantErrs: []validationMatch{ invalid("Invalid value: \"0\": failed rule: self > oldSelf", - "spec.topology.workers.machineDeployments[0].variables.overrides[cpu].value"), + "spec.topology.workers.machineDeployments[mdTopologyName].variables.overrides[cpu].value"), }, definitions: []clusterv1.ClusterClassStatusVariable{ { @@ -2661,7 +2661,7 @@ func Test_ValidateMachineVariables(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { gotErrs := ValidateMachineVariables(ctx, tt.values, tt.oldValues, tt.definitions, - field.NewPath("spec", "topology", "workers", "machineDeployments").Index(0).Child("variables", "overrides")) + field.NewPath("spec", "topology", "workers", "machineDeployments").Key("mdTopologyName").Child("variables", "overrides")) checkErrors(t, tt.wantErrs, gotErrs) }) diff --git a/internal/webhooks/cluster.go b/internal/webhooks/cluster.go index 3b9c49be20d5..f2baec9e612f 100644 --- a/internal/webhooks/cluster.go +++ b/internal/webhooks/cluster.go @@ -676,7 +676,7 @@ func validateMachineHealthChecks(cluster *clusterv1.Cluster, clusterClass *clust for i := range cluster.Spec.Topology.Workers.MachineDeployments { md := cluster.Spec.Topology.Workers.MachineDeployments[i] if md.MachineHealthCheck != nil { - fldPath := field.NewPath("spec", "topology", "workers", "machineDeployments", "machineHealthCheck").Index(i) + fldPath := field.NewPath("spec", "topology", "workers", "machineDeployments").Key(md.Name).Child("machineHealthCheck") // Validate the MachineDeployment MachineHealthCheck if defined. if !md.MachineHealthCheck.MachineHealthCheckClass.IsZero() { @@ -791,7 +791,7 @@ func DefaultAndValidateVariables(ctx context.Context, cluster, oldCluster *clust if cluster.Spec.Topology.Workers != nil { // Validate MachineDeployment variable overrides. - for i, md := range cluster.Spec.Topology.Workers.MachineDeployments { + for _, md := range cluster.Spec.Topology.Workers.MachineDeployments { // Continue if there are no variable overrides. if md.Variables == nil || len(md.Variables.Overrides) == 0 { continue @@ -801,12 +801,12 @@ func DefaultAndValidateVariables(ctx context.Context, cluster, oldCluster *clust md.Variables.Overrides, oldMDVariables[md.Name], clusterClass.Status.Variables, - field.NewPath("spec", "topology", "workers", "machineDeployments").Index(i).Child("variables", "overrides"))..., + field.NewPath("spec", "topology", "workers", "machineDeployments").Key(md.Name).Child("variables", "overrides"))..., ) } // Validate MachinePool variable overrides. - for i, mp := range cluster.Spec.Topology.Workers.MachinePools { + for _, mp := range cluster.Spec.Topology.Workers.MachinePools { // Continue if there are no variable overrides. if mp.Variables == nil || len(mp.Variables.Overrides) == 0 { continue @@ -816,7 +816,7 @@ func DefaultAndValidateVariables(ctx context.Context, cluster, oldCluster *clust mp.Variables.Overrides, oldMPVariables[mp.Name], clusterClass.Status.Variables, - field.NewPath("spec", "topology", "workers", "machinePools").Index(i).Child("variables", "overrides"))..., + field.NewPath("spec", "topology", "workers", "machinePools").Key(mp.Name).Child("variables", "overrides"))..., ) } } @@ -855,13 +855,13 @@ func DefaultVariables(cluster *clusterv1.Cluster, clusterClass *clusterv1.Cluste if cluster.Spec.Topology.Workers != nil { // Default MachineDeployment variable overrides. - for i, md := range cluster.Spec.Topology.Workers.MachineDeployments { + for _, md := range cluster.Spec.Topology.Workers.MachineDeployments { // Continue if there are no variable overrides. if md.Variables == nil || len(md.Variables.Overrides) == 0 { continue } defaultedVariables, errs := variables.DefaultMachineVariables(md.Variables.Overrides, clusterClass.Status.Variables, - field.NewPath("spec", "topology", "workers", "machineDeployments").Index(i).Child("variables", "overrides")) + field.NewPath("spec", "topology", "workers", "machineDeployments").Key(md.Name).Child("variables", "overrides")) if len(errs) > 0 { allErrs = append(allErrs, errs...) } else { @@ -870,13 +870,13 @@ func DefaultVariables(cluster *clusterv1.Cluster, clusterClass *clusterv1.Cluste } // Default MachinePool variable overrides. - for i, mp := range cluster.Spec.Topology.Workers.MachinePools { + for _, mp := range cluster.Spec.Topology.Workers.MachinePools { // Continue if there are no variable overrides. if mp.Variables == nil || len(mp.Variables.Overrides) == 0 { continue } defaultedVariables, errs := variables.DefaultMachineVariables(mp.Variables.Overrides, clusterClass.Status.Variables, - field.NewPath("spec", "topology", "workers", "machinePools").Index(i).Child("variables", "overrides")) + field.NewPath("spec", "topology", "workers", "machinePools").Key(mp.Name).Child("variables", "overrides")) if len(errs) > 0 { allErrs = append(allErrs, errs...) } else { @@ -978,14 +978,14 @@ func validateTopologyMetadata(topology *clusterv1.Topology, fldPath *field.Path) var allErrs field.ErrorList allErrs = append(allErrs, topology.ControlPlane.Metadata.Validate(fldPath.Child("controlPlane", "metadata"))...) if topology.Workers != nil { - for idx, md := range topology.Workers.MachineDeployments { + for _, md := range topology.Workers.MachineDeployments { allErrs = append(allErrs, md.Metadata.Validate( - fldPath.Child("workers", "machineDeployments").Index(idx).Child("metadata"), + fldPath.Child("workers", "machineDeployments").Key(md.Name).Child("metadata"), )...) } - for idx, mp := range topology.Workers.MachinePools { + for _, mp := range topology.Workers.MachinePools { allErrs = append(allErrs, mp.Metadata.Validate( - fldPath.Child("workers", "machinePools").Index(idx).Child("metadata"), + fldPath.Child("workers", "machinePools").Key(mp.Name).Child("metadata"), )...) } } @@ -1003,7 +1003,7 @@ func validateAutoscalerAnnotationsForCluster(cluster *clusterv1.Cluster, cluster } fldPath := field.NewPath("spec", "topology") - for i, mdt := range cluster.Spec.Topology.Workers.MachineDeployments { + for _, mdt := range cluster.Spec.Topology.Workers.MachineDeployments { if mdt.Replicas == nil { continue } @@ -1012,8 +1012,8 @@ func validateAutoscalerAnnotationsForCluster(cluster *clusterv1.Cluster, cluster allErrs = append( allErrs, field.Invalid( - fldPath.Child("workers", "machineDeployments").Index(i).Child("replicas"), - cluster.Spec.Topology.Workers.MachineDeployments[i].Replicas, + fldPath.Child("workers", "machineDeployments").Key(mdt.Name).Child("replicas"), + mdt.Replicas, fmt.Sprintf("cannot be set for cluster %q in namespace %q if the same MachineDeploymentTopology has autoscaler annotations", cluster.Name, cluster.Namespace), ), @@ -1036,8 +1036,8 @@ func validateAutoscalerAnnotationsForCluster(cluster *clusterv1.Cluster, cluster allErrs = append( allErrs, field.Invalid( - fldPath.Child("workers", "machineDeployments").Index(i).Child("replicas"), - cluster.Spec.Topology.Workers.MachineDeployments[i].Replicas, + fldPath.Child("workers", "machineDeployments").Key(mdt.Name).Child("replicas"), + mdt.Replicas, fmt.Sprintf("cannot be set for cluster %q in namespace %q if the source class %q of this MachineDeploymentTopology has autoscaler annotations", cluster.Name, cluster.Namespace, mdt.Class), ), diff --git a/internal/webhooks/cluster_test.go b/internal/webhooks/cluster_test.go index 3d58c29cc411..3a78d45cb70d 100644 --- a/internal/webhooks/cluster_test.go +++ b/internal/webhooks/cluster_test.go @@ -1190,8 +1190,8 @@ func TestClusterDefaultAndValidateVariables(t *testing.T) { wantErrMessage: "Cluster.cluster.x-k8s.io \"cluster1\" is invalid: [" + "spec.topology.variables[cpu].value: Invalid value: \"-5\": failed rule: self > oldSelf, " + "spec.topology.controlPlane.variables.overrides[cpu].value: Invalid value: \"-4\": failed rule: self > oldSelf, " + - "spec.topology.workers.machineDeployments[0].variables.overrides[cpu].value: Invalid value: \"-10\": failed rule: self > oldSelf, " + - "spec.topology.workers.machinePools[0].variables.overrides[cpu].value: Invalid value: \"-11\": failed rule: self > oldSelf]", + "spec.topology.workers.machineDeployments[workers1].variables.overrides[cpu].value: Invalid value: \"-10\": failed rule: self > oldSelf, " + + "spec.topology.workers.machinePools[workers1].variables.overrides[cpu].value: Invalid value: \"-11\": failed rule: self > oldSelf]", }, } for _, tt := range tests { diff --git a/internal/webhooks/clusterclass.go b/internal/webhooks/clusterclass.go index db5ec80dadc7..3025acceb436 100644 --- a/internal/webhooks/clusterclass.go +++ b/internal/webhooks/clusterclass.go @@ -241,7 +241,7 @@ func validateUpdatesToMachineHealthCheckClasses(clusters []clusterv1.Cluster, ol } // For each MachineDeploymentClass check if the MachineHealthCheck definition is dropped. - for i, newMdClass := range newClusterClass.Spec.Workers.MachineDeployments { + for _, newMdClass := range newClusterClass.Spec.Workers.MachineDeployments { oldMdClass := machineDeploymentClassOfName(oldClusterClass, newMdClass.Class) if oldMdClass == nil { // This is a new MachineDeploymentClass. Nothing to do here. @@ -268,7 +268,7 @@ func validateUpdatesToMachineHealthCheckClasses(clusters []clusterv1.Cluster, ol } if len(clustersUsingMHC) != 0 { allErrs = append(allErrs, field.Forbidden( - field.NewPath("spec", "workers", "machineDeployments").Index(i).Child("machineHealthCheck"), + field.NewPath("spec", "workers", "machineDeployments").Key(newMdClass.Class).Child("machineHealthCheck"), fmt.Sprintf("MachineHealthCheck cannot be deleted because it is used by Cluster(s) %q", strings.Join(clustersUsingMHC, ",")), )) } @@ -411,11 +411,11 @@ func validateMachineHealthCheckClasses(clusterClass *clusterv1.ClusterClass) fie } // Validate MachineDeployment MachineHealthChecks. - for i, md := range clusterClass.Spec.Workers.MachineDeployments { + for _, md := range clusterClass.Spec.Workers.MachineDeployments { if md.MachineHealthCheck == nil { continue } - fldPath := field.NewPath("spec", "workers", "machineDeployments").Index(i).Child("machineHealthCheck") + fldPath := field.NewPath("spec", "workers", "machineDeployments").Key(md.Class).Child("machineHealthCheck") allErrs = append(allErrs, validateMachineHealthCheckClass(fldPath, clusterClass.Namespace, md.MachineHealthCheck)...) } @@ -442,12 +442,12 @@ func validateNamingStrategies(clusterClass *clusterv1.ClusterClass) field.ErrorL } } - for i, md := range clusterClass.Spec.Workers.MachineDeployments { + for _, md := range clusterClass.Spec.Workers.MachineDeployments { if md.NamingStrategy == nil || md.NamingStrategy.Template == nil { continue } name, err := names.MachineDeploymentNameGenerator(*md.NamingStrategy.Template, "cluster", "mdtopology").GenerateName() - templateFldPath := field.NewPath("spec", "workers", "machineDeployments").Index(i).Child("namingStrategy", "template") + templateFldPath := field.NewPath("spec", "workers", "machineDeployments").Key(md.Class).Child("namingStrategy", "template") if err != nil { allErrs = append(allErrs, field.Invalid( @@ -462,12 +462,12 @@ func validateNamingStrategies(clusterClass *clusterv1.ClusterClass) field.ErrorL } } - for i, mp := range clusterClass.Spec.Workers.MachinePools { + for _, mp := range clusterClass.Spec.Workers.MachinePools { if mp.NamingStrategy == nil || mp.NamingStrategy.Template == nil { continue } name, err := names.MachinePoolNameGenerator(*mp.NamingStrategy.Template, "cluster", "mptopology").GenerateName() - templateFldPath := field.NewPath("spec", "workers", "machinePools").Index(i).Child("namingStrategy", "template") + templateFldPath := field.NewPath("spec", "workers", "machinePools").Key(mp.Class).Child("namingStrategy", "template") if err != nil { allErrs = append(allErrs, field.Invalid( @@ -505,11 +505,11 @@ func validateMachineHealthCheckClass(fldPath *field.Path, namepace string, m *cl func validateClusterClassMetadata(clusterClass *clusterv1.ClusterClass) field.ErrorList { var allErrs field.ErrorList allErrs = append(allErrs, clusterClass.Spec.ControlPlane.Metadata.Validate(field.NewPath("spec", "controlPlane", "metadata"))...) - for idx, m := range clusterClass.Spec.Workers.MachineDeployments { - allErrs = append(allErrs, m.Template.Metadata.Validate(field.NewPath("spec", "workers", "machineDeployments").Index(idx).Child("template", "metadata"))...) + for _, m := range clusterClass.Spec.Workers.MachineDeployments { + allErrs = append(allErrs, m.Template.Metadata.Validate(field.NewPath("spec", "workers", "machineDeployments").Key(m.Class).Child("template", "metadata"))...) } - for idx, m := range clusterClass.Spec.Workers.MachinePools { - allErrs = append(allErrs, m.Template.Metadata.Validate(field.NewPath("spec", "workers", "machinePools").Index(idx).Child("template", "metadata"))...) + for _, m := range clusterClass.Spec.Workers.MachinePools { + allErrs = append(allErrs, m.Template.Metadata.Validate(field.NewPath("spec", "workers", "machinePools").Key(m.Class).Child("template", "metadata"))...) } return allErrs }