From 219b996ec599768d3541e2e0c1e05f33028e168a Mon Sep 17 00:00:00 2001 From: Stefan Bueringer Date: Tue, 13 May 2025 18:47:05 +0200 Subject: [PATCH] Remove ClusterVariable.DefinitionFrom field --- api/v1beta1/conversion.go | 6 + api/v1beta1/conversion_test.go | 8 + api/v1beta1/zz_generated.conversion.go | 262 +++++++++++++++--- api/v1beta2/cluster_types.go | 8 - api/v1beta2/zz_generated.openapi.go | 7 - .../crd/bases/cluster.x-k8s.io_clusters.yaml | 28 -- .../topology/cluster/patches/engine.go | 2 +- .../cluster_variable_defaulting_test.go | 48 ---- .../cluster_variable_validation_test.go | 114 -------- internal/topology/variables/utils.go | 9 +- internal/webhooks/cluster.go | 59 ---- internal/webhooks/cluster_test.go | 68 ----- 12 files changed, 244 insertions(+), 375 deletions(-) diff --git a/api/v1beta1/conversion.go b/api/v1beta1/conversion.go index 3a7a039877e1..9166d181c4fc 100644 --- a/api/v1beta1/conversion.go +++ b/api/v1beta1/conversion.go @@ -573,3 +573,9 @@ func Convert_v1beta1_Condition_To_v1_Condition(_ *Condition, _ *metav1.Condition // NOTE: legacy (v1beta1) conditions should not be automatically converted into v1beta2 conditions. return nil } + +func Convert_v1beta1_ClusterVariable_To_v1beta2_ClusterVariable(in *ClusterVariable, out *clusterv1.ClusterVariable, s apimachineryconversion.Scope) error { + // NOTE: v1beta2 ClusterVariable does not have DefinitionFrom anymore. But it's fine to just lose this field, + // because it was already not possible to set it anymore with v1beta1. + return autoConvert_v1beta1_ClusterVariable_To_v1beta2_ClusterVariable(in, out, s) +} diff --git a/api/v1beta1/conversion_test.go b/api/v1beta1/conversion_test.go index 97b61f57ecfd..c6bf78f2b367 100644 --- a/api/v1beta1/conversion_test.go +++ b/api/v1beta1/conversion_test.go @@ -72,6 +72,7 @@ func ClusterFuzzFuncs(_ runtimeserializer.CodecFactory) []interface{} { return []interface{}{ hubClusterStatus, spokeClusterStatus, + spokeClusterVariable, } } @@ -102,6 +103,13 @@ func spokeClusterStatus(in *ClusterStatus, c fuzz.Continue) { } } +func spokeClusterVariable(in *ClusterVariable, c fuzz.Continue) { + c.FuzzNoCustom(in) + + // Drop DefinitionFrom as we intentionally don't preserve it. + in.DefinitionFrom = "" +} + func ClusterClassFuncs(_ runtimeserializer.CodecFactory) []interface{} { return []interface{}{ hubClusterClassStatus, diff --git a/api/v1beta1/zz_generated.conversion.go b/api/v1beta1/zz_generated.conversion.go index d97375e884f0..de5542d4d5f8 100644 --- a/api/v1beta1/zz_generated.conversion.go +++ b/api/v1beta1/zz_generated.conversion.go @@ -200,11 +200,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*ClusterVariable)(nil), (*v1beta2.ClusterVariable)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1beta1_ClusterVariable_To_v1beta2_ClusterVariable(a.(*ClusterVariable), b.(*v1beta2.ClusterVariable), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*v1beta2.ClusterVariable)(nil), (*ClusterVariable)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1beta2_ClusterVariable_To_v1beta1_ClusterVariable(a.(*v1beta2.ClusterVariable), b.(*ClusterVariable), scope) }); err != nil { @@ -875,6 +870,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*ClusterVariable)(nil), (*v1beta2.ClusterVariable)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1beta1_ClusterVariable_To_v1beta2_ClusterVariable(a.(*ClusterVariable), b.(*v1beta2.ClusterVariable), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*Condition)(nil), (*v1.Condition)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1beta1_Condition_To_v1_Condition(a.(*Condition), b.(*v1.Condition), scope) }); err != nil { @@ -1431,7 +1431,15 @@ func autoConvert_v1beta1_ClusterSpec_To_v1beta2_ClusterSpec(in *ClusterSpec, out } out.ControlPlaneRef = (*corev1.ObjectReference)(unsafe.Pointer(in.ControlPlaneRef)) out.InfrastructureRef = (*corev1.ObjectReference)(unsafe.Pointer(in.InfrastructureRef)) - out.Topology = (*v1beta2.Topology)(unsafe.Pointer(in.Topology)) + if in.Topology != nil { + in, out := &in.Topology, &out.Topology + *out = new(v1beta2.Topology) + if err := Convert_v1beta1_Topology_To_v1beta2_Topology(*in, *out, s); err != nil { + return err + } + } else { + out.Topology = nil + } out.AvailabilityGates = *(*[]v1beta2.ClusterAvailabilityGate)(unsafe.Pointer(&in.AvailabilityGates)) return nil } @@ -1449,7 +1457,15 @@ func autoConvert_v1beta2_ClusterSpec_To_v1beta1_ClusterSpec(in *v1beta2.ClusterS } out.ControlPlaneRef = (*corev1.ObjectReference)(unsafe.Pointer(in.ControlPlaneRef)) out.InfrastructureRef = (*corev1.ObjectReference)(unsafe.Pointer(in.InfrastructureRef)) - out.Topology = (*Topology)(unsafe.Pointer(in.Topology)) + if in.Topology != nil { + in, out := &in.Topology, &out.Topology + *out = new(Topology) + if err := Convert_v1beta2_Topology_To_v1beta1_Topology(*in, *out, s); err != nil { + return err + } + } else { + out.Topology = nil + } out.AvailabilityGates = *(*[]ClusterAvailabilityGate)(unsafe.Pointer(&in.AvailabilityGates)) return nil } @@ -1506,19 +1522,13 @@ func autoConvert_v1beta2_ClusterStatus_To_v1beta1_ClusterStatus(in *v1beta2.Clus func autoConvert_v1beta1_ClusterVariable_To_v1beta2_ClusterVariable(in *ClusterVariable, out *v1beta2.ClusterVariable, s conversion.Scope) error { out.Name = in.Name - out.DefinitionFrom = in.DefinitionFrom + // WARNING: in.DefinitionFrom requires manual conversion: does not exist in peer-type out.Value = in.Value return nil } -// Convert_v1beta1_ClusterVariable_To_v1beta2_ClusterVariable is an autogenerated conversion function. -func Convert_v1beta1_ClusterVariable_To_v1beta2_ClusterVariable(in *ClusterVariable, out *v1beta2.ClusterVariable, s conversion.Scope) error { - return autoConvert_v1beta1_ClusterVariable_To_v1beta2_ClusterVariable(in, out, s) -} - func autoConvert_v1beta2_ClusterVariable_To_v1beta1_ClusterVariable(in *v1beta2.ClusterVariable, out *ClusterVariable, s conversion.Scope) error { out.Name = in.Name - out.DefinitionFrom = in.DefinitionFrom out.Value = in.Value return nil } @@ -1632,7 +1642,15 @@ func autoConvert_v1beta1_ControlPlaneTopology_To_v1beta2_ControlPlaneTopology(in out.NodeVolumeDetachTimeout = (*v1.Duration)(unsafe.Pointer(in.NodeVolumeDetachTimeout)) out.NodeDeletionTimeout = (*v1.Duration)(unsafe.Pointer(in.NodeDeletionTimeout)) out.ReadinessGates = *(*[]v1beta2.MachineReadinessGate)(unsafe.Pointer(&in.ReadinessGates)) - out.Variables = (*v1beta2.ControlPlaneVariables)(unsafe.Pointer(in.Variables)) + if in.Variables != nil { + in, out := &in.Variables, &out.Variables + *out = new(v1beta2.ControlPlaneVariables) + if err := Convert_v1beta1_ControlPlaneVariables_To_v1beta2_ControlPlaneVariables(*in, *out, s); err != nil { + return err + } + } else { + out.Variables = nil + } return nil } @@ -1651,7 +1669,15 @@ func autoConvert_v1beta2_ControlPlaneTopology_To_v1beta1_ControlPlaneTopology(in out.NodeVolumeDetachTimeout = (*v1.Duration)(unsafe.Pointer(in.NodeVolumeDetachTimeout)) out.NodeDeletionTimeout = (*v1.Duration)(unsafe.Pointer(in.NodeDeletionTimeout)) out.ReadinessGates = *(*[]MachineReadinessGate)(unsafe.Pointer(&in.ReadinessGates)) - out.Variables = (*ControlPlaneVariables)(unsafe.Pointer(in.Variables)) + if in.Variables != nil { + in, out := &in.Variables, &out.Variables + *out = new(ControlPlaneVariables) + if err := Convert_v1beta2_ControlPlaneVariables_To_v1beta1_ControlPlaneVariables(*in, *out, s); err != nil { + return err + } + } else { + out.Variables = nil + } return nil } @@ -1661,7 +1687,17 @@ func Convert_v1beta2_ControlPlaneTopology_To_v1beta1_ControlPlaneTopology(in *v1 } func autoConvert_v1beta1_ControlPlaneVariables_To_v1beta2_ControlPlaneVariables(in *ControlPlaneVariables, out *v1beta2.ControlPlaneVariables, s conversion.Scope) error { - out.Overrides = *(*[]v1beta2.ClusterVariable)(unsafe.Pointer(&in.Overrides)) + if in.Overrides != nil { + in, out := &in.Overrides, &out.Overrides + *out = make([]v1beta2.ClusterVariable, len(*in)) + for i := range *in { + if err := Convert_v1beta1_ClusterVariable_To_v1beta2_ClusterVariable(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.Overrides = nil + } return nil } @@ -1671,7 +1707,17 @@ func Convert_v1beta1_ControlPlaneVariables_To_v1beta2_ControlPlaneVariables(in * } func autoConvert_v1beta2_ControlPlaneVariables_To_v1beta1_ControlPlaneVariables(in *v1beta2.ControlPlaneVariables, out *ControlPlaneVariables, s conversion.Scope) error { - out.Overrides = *(*[]ClusterVariable)(unsafe.Pointer(&in.Overrides)) + if in.Overrides != nil { + in, out := &in.Overrides, &out.Overrides + *out = make([]ClusterVariable, len(*in)) + for i := range *in { + if err := Convert_v1beta2_ClusterVariable_To_v1beta1_ClusterVariable(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.Overrides = nil + } return nil } @@ -2282,7 +2328,15 @@ func autoConvert_v1beta1_MachineDeploymentTopology_To_v1beta2_MachineDeploymentT out.MinReadySeconds = (*int32)(unsafe.Pointer(in.MinReadySeconds)) out.ReadinessGates = *(*[]v1beta2.MachineReadinessGate)(unsafe.Pointer(&in.ReadinessGates)) out.Strategy = (*v1beta2.MachineDeploymentStrategy)(unsafe.Pointer(in.Strategy)) - out.Variables = (*v1beta2.MachineDeploymentVariables)(unsafe.Pointer(in.Variables)) + if in.Variables != nil { + in, out := &in.Variables, &out.Variables + *out = new(v1beta2.MachineDeploymentVariables) + if err := Convert_v1beta1_MachineDeploymentVariables_To_v1beta2_MachineDeploymentVariables(*in, *out, s); err != nil { + return err + } + } else { + out.Variables = nil + } return nil } @@ -2306,7 +2360,15 @@ func autoConvert_v1beta2_MachineDeploymentTopology_To_v1beta1_MachineDeploymentT out.MinReadySeconds = (*int32)(unsafe.Pointer(in.MinReadySeconds)) out.ReadinessGates = *(*[]MachineReadinessGate)(unsafe.Pointer(&in.ReadinessGates)) out.Strategy = (*MachineDeploymentStrategy)(unsafe.Pointer(in.Strategy)) - out.Variables = (*MachineDeploymentVariables)(unsafe.Pointer(in.Variables)) + if in.Variables != nil { + in, out := &in.Variables, &out.Variables + *out = new(MachineDeploymentVariables) + if err := Convert_v1beta2_MachineDeploymentVariables_To_v1beta1_MachineDeploymentVariables(*in, *out, s); err != nil { + return err + } + } else { + out.Variables = nil + } return nil } @@ -2316,7 +2378,17 @@ func Convert_v1beta2_MachineDeploymentTopology_To_v1beta1_MachineDeploymentTopol } func autoConvert_v1beta1_MachineDeploymentVariables_To_v1beta2_MachineDeploymentVariables(in *MachineDeploymentVariables, out *v1beta2.MachineDeploymentVariables, s conversion.Scope) error { - out.Overrides = *(*[]v1beta2.ClusterVariable)(unsafe.Pointer(&in.Overrides)) + if in.Overrides != nil { + in, out := &in.Overrides, &out.Overrides + *out = make([]v1beta2.ClusterVariable, len(*in)) + for i := range *in { + if err := Convert_v1beta1_ClusterVariable_To_v1beta2_ClusterVariable(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.Overrides = nil + } return nil } @@ -2326,7 +2398,17 @@ func Convert_v1beta1_MachineDeploymentVariables_To_v1beta2_MachineDeploymentVari } func autoConvert_v1beta2_MachineDeploymentVariables_To_v1beta1_MachineDeploymentVariables(in *v1beta2.MachineDeploymentVariables, out *MachineDeploymentVariables, s conversion.Scope) error { - out.Overrides = *(*[]ClusterVariable)(unsafe.Pointer(&in.Overrides)) + if in.Overrides != nil { + in, out := &in.Overrides, &out.Overrides + *out = make([]ClusterVariable, len(*in)) + for i := range *in { + if err := Convert_v1beta2_ClusterVariable_To_v1beta1_ClusterVariable(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.Overrides = nil + } return nil } @@ -2847,7 +2929,15 @@ func autoConvert_v1beta1_MachinePoolTopology_To_v1beta2_MachinePoolTopology(in * out.NodeDeletionTimeout = (*v1.Duration)(unsafe.Pointer(in.NodeDeletionTimeout)) out.MinReadySeconds = (*int32)(unsafe.Pointer(in.MinReadySeconds)) out.Replicas = (*int32)(unsafe.Pointer(in.Replicas)) - out.Variables = (*v1beta2.MachinePoolVariables)(unsafe.Pointer(in.Variables)) + if in.Variables != nil { + in, out := &in.Variables, &out.Variables + *out = new(v1beta2.MachinePoolVariables) + if err := Convert_v1beta1_MachinePoolVariables_To_v1beta2_MachinePoolVariables(*in, *out, s); err != nil { + return err + } + } else { + out.Variables = nil + } return nil } @@ -2868,7 +2958,15 @@ func autoConvert_v1beta2_MachinePoolTopology_To_v1beta1_MachinePoolTopology(in * out.NodeDeletionTimeout = (*v1.Duration)(unsafe.Pointer(in.NodeDeletionTimeout)) out.MinReadySeconds = (*int32)(unsafe.Pointer(in.MinReadySeconds)) out.Replicas = (*int32)(unsafe.Pointer(in.Replicas)) - out.Variables = (*MachinePoolVariables)(unsafe.Pointer(in.Variables)) + if in.Variables != nil { + in, out := &in.Variables, &out.Variables + *out = new(MachinePoolVariables) + if err := Convert_v1beta2_MachinePoolVariables_To_v1beta1_MachinePoolVariables(*in, *out, s); err != nil { + return err + } + } else { + out.Variables = nil + } return nil } @@ -2878,7 +2976,17 @@ func Convert_v1beta2_MachinePoolTopology_To_v1beta1_MachinePoolTopology(in *v1be } func autoConvert_v1beta1_MachinePoolVariables_To_v1beta2_MachinePoolVariables(in *MachinePoolVariables, out *v1beta2.MachinePoolVariables, s conversion.Scope) error { - out.Overrides = *(*[]v1beta2.ClusterVariable)(unsafe.Pointer(&in.Overrides)) + if in.Overrides != nil { + in, out := &in.Overrides, &out.Overrides + *out = make([]v1beta2.ClusterVariable, len(*in)) + for i := range *in { + if err := Convert_v1beta1_ClusterVariable_To_v1beta2_ClusterVariable(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.Overrides = nil + } return nil } @@ -2888,7 +2996,17 @@ func Convert_v1beta1_MachinePoolVariables_To_v1beta2_MachinePoolVariables(in *Ma } func autoConvert_v1beta2_MachinePoolVariables_To_v1beta1_MachinePoolVariables(in *v1beta2.MachinePoolVariables, out *MachinePoolVariables, s conversion.Scope) error { - out.Overrides = *(*[]ClusterVariable)(unsafe.Pointer(&in.Overrides)) + if in.Overrides != nil { + in, out := &in.Overrides, &out.Overrides + *out = make([]ClusterVariable, len(*in)) + for i := range *in { + if err := Convert_v1beta2_ClusterVariable_To_v1beta1_ClusterVariable(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.Overrides = nil + } return nil } @@ -3422,8 +3540,26 @@ func autoConvert_v1beta1_Topology_To_v1beta2_Topology(in *Topology, out *v1beta2 if err := Convert_v1beta1_ControlPlaneTopology_To_v1beta2_ControlPlaneTopology(&in.ControlPlane, &out.ControlPlane, s); err != nil { return err } - out.Workers = (*v1beta2.WorkersTopology)(unsafe.Pointer(in.Workers)) - out.Variables = *(*[]v1beta2.ClusterVariable)(unsafe.Pointer(&in.Variables)) + if in.Workers != nil { + in, out := &in.Workers, &out.Workers + *out = new(v1beta2.WorkersTopology) + if err := Convert_v1beta1_WorkersTopology_To_v1beta2_WorkersTopology(*in, *out, s); err != nil { + return err + } + } else { + out.Workers = nil + } + if in.Variables != nil { + in, out := &in.Variables, &out.Variables + *out = make([]v1beta2.ClusterVariable, len(*in)) + for i := range *in { + if err := Convert_v1beta1_ClusterVariable_To_v1beta2_ClusterVariable(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.Variables = nil + } return nil } @@ -3440,8 +3576,26 @@ func autoConvert_v1beta2_Topology_To_v1beta1_Topology(in *v1beta2.Topology, out if err := Convert_v1beta2_ControlPlaneTopology_To_v1beta1_ControlPlaneTopology(&in.ControlPlane, &out.ControlPlane, s); err != nil { return err } - out.Workers = (*WorkersTopology)(unsafe.Pointer(in.Workers)) - out.Variables = *(*[]ClusterVariable)(unsafe.Pointer(&in.Variables)) + if in.Workers != nil { + in, out := &in.Workers, &out.Workers + *out = new(WorkersTopology) + if err := Convert_v1beta2_WorkersTopology_To_v1beta1_WorkersTopology(*in, *out, s); err != nil { + return err + } + } else { + out.Workers = nil + } + if in.Variables != nil { + in, out := &in.Variables, &out.Variables + *out = make([]ClusterVariable, len(*in)) + for i := range *in { + if err := Convert_v1beta2_ClusterVariable_To_v1beta1_ClusterVariable(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.Variables = nil + } return nil } @@ -3599,8 +3753,28 @@ func Convert_v1beta2_WorkersStatus_To_v1beta1_WorkersStatus(in *v1beta2.WorkersS } func autoConvert_v1beta1_WorkersTopology_To_v1beta2_WorkersTopology(in *WorkersTopology, out *v1beta2.WorkersTopology, s conversion.Scope) error { - out.MachineDeployments = *(*[]v1beta2.MachineDeploymentTopology)(unsafe.Pointer(&in.MachineDeployments)) - out.MachinePools = *(*[]v1beta2.MachinePoolTopology)(unsafe.Pointer(&in.MachinePools)) + if in.MachineDeployments != nil { + in, out := &in.MachineDeployments, &out.MachineDeployments + *out = make([]v1beta2.MachineDeploymentTopology, len(*in)) + for i := range *in { + if err := Convert_v1beta1_MachineDeploymentTopology_To_v1beta2_MachineDeploymentTopology(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.MachineDeployments = nil + } + if in.MachinePools != nil { + in, out := &in.MachinePools, &out.MachinePools + *out = make([]v1beta2.MachinePoolTopology, len(*in)) + for i := range *in { + if err := Convert_v1beta1_MachinePoolTopology_To_v1beta2_MachinePoolTopology(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.MachinePools = nil + } return nil } @@ -3610,8 +3784,28 @@ func Convert_v1beta1_WorkersTopology_To_v1beta2_WorkersTopology(in *WorkersTopol } func autoConvert_v1beta2_WorkersTopology_To_v1beta1_WorkersTopology(in *v1beta2.WorkersTopology, out *WorkersTopology, s conversion.Scope) error { - out.MachineDeployments = *(*[]MachineDeploymentTopology)(unsafe.Pointer(&in.MachineDeployments)) - out.MachinePools = *(*[]MachinePoolTopology)(unsafe.Pointer(&in.MachinePools)) + if in.MachineDeployments != nil { + in, out := &in.MachineDeployments, &out.MachineDeployments + *out = make([]MachineDeploymentTopology, len(*in)) + for i := range *in { + if err := Convert_v1beta2_MachineDeploymentTopology_To_v1beta1_MachineDeploymentTopology(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.MachineDeployments = nil + } + if in.MachinePools != nil { + in, out := &in.MachinePools, &out.MachinePools + *out = make([]MachinePoolTopology, len(*in)) + for i := range *in { + if err := Convert_v1beta2_MachinePoolTopology_To_v1beta1_MachinePoolTopology(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.MachinePools = nil + } return nil } diff --git a/api/v1beta2/cluster_types.go b/api/v1beta2/cluster_types.go index c48ff2f8e36e..7edffd6dbd6d 100644 --- a/api/v1beta2/cluster_types.go +++ b/api/v1beta2/cluster_types.go @@ -852,14 +852,6 @@ type ClusterVariable struct { // +kubebuilder:validation:MaxLength=256 Name string `json:"name"` - // definitionFrom specifies where the definition of this Variable is from. - // - // Deprecated: This field is deprecated, must not be set anymore and is going to be removed in the next apiVersion. - // - // +optional - // +kubebuilder:validation:MaxLength=256 - DefinitionFrom string `json:"definitionFrom,omitempty"` - // value of the variable. // Note: the value will be validated against the schema of the corresponding ClusterClassVariable // from the ClusterClass. diff --git a/api/v1beta2/zz_generated.openapi.go b/api/v1beta2/zz_generated.openapi.go index 1f585c6fbfbe..06c160d93d97 100644 --- a/api/v1beta2/zz_generated.openapi.go +++ b/api/v1beta2/zz_generated.openapi.go @@ -1231,13 +1231,6 @@ func schema_sigsk8sio_cluster_api_api_v1beta2_ClusterVariable(ref common.Referen Format: "", }, }, - "definitionFrom": { - SchemaProps: spec.SchemaProps{ - Description: "definitionFrom specifies where the definition of this Variable is from.\n\nDeprecated: This field is deprecated, must not be set anymore and is going to be removed in the next apiVersion.", - Type: []string{"string"}, - Format: "", - }, - }, "value": { SchemaProps: spec.SchemaProps{ Description: "value of the variable. Note: the value will be validated against the schema of the corresponding ClusterClassVariable from the ClusterClass. Note: We have to use apiextensionsv1.JSON instead of a custom JSON type, because controller-tools has a hard-coded schema for apiextensionsv1.JSON which cannot be produced by another type via controller-tools, i.e. it is not possible to have no type field. Ref: https://github.com/kubernetes-sigs/controller-tools/blob/d0e03a142d0ecdd5491593e941ee1d6b5d91dba6/pkg/crd/known_types.go#L106-L111", diff --git a/config/crd/bases/cluster.x-k8s.io_clusters.yaml b/config/crd/bases/cluster.x-k8s.io_clusters.yaml index f1e5cc17e5e9..fc0273e2f9d6 100644 --- a/config/crd/bases/cluster.x-k8s.io_clusters.yaml +++ b/config/crd/bases/cluster.x-k8s.io_clusters.yaml @@ -2649,13 +2649,6 @@ spec: ClusterVariable can be used to customize the Cluster through patches. Each ClusterVariable is associated with a Variable definition in the ClusterClass `status` variables. properties: - definitionFrom: - description: |- - definitionFrom specifies where the definition of this Variable is from. - - Deprecated: This field is deprecated, must not be set anymore and is going to be removed in the next apiVersion. - maxLength: 256 - type: string name: description: name of the variable. maxLength: 256 @@ -2700,13 +2693,6 @@ spec: ClusterVariable can be used to customize the Cluster through patches. Each ClusterVariable is associated with a Variable definition in the ClusterClass `status` variables. properties: - definitionFrom: - description: |- - definitionFrom specifies where the definition of this Variable is from. - - Deprecated: This field is deprecated, must not be set anymore and is going to be removed in the next apiVersion. - maxLength: 256 - type: string name: description: name of the variable. maxLength: 256 @@ -3115,13 +3101,6 @@ spec: ClusterVariable can be used to customize the Cluster through patches. Each ClusterVariable is associated with a Variable definition in the ClusterClass `status` variables. properties: - definitionFrom: - description: |- - definitionFrom specifies where the definition of this Variable is from. - - Deprecated: This field is deprecated, must not be set anymore and is going to be removed in the next apiVersion. - maxLength: 256 - type: string name: description: name of the variable. maxLength: 256 @@ -3260,13 +3239,6 @@ spec: ClusterVariable can be used to customize the Cluster through patches. Each ClusterVariable is associated with a Variable definition in the ClusterClass `status` variables. properties: - definitionFrom: - description: |- - definitionFrom specifies where the definition of this Variable is from. - - Deprecated: This field is deprecated, must not be set anymore and is going to be removed in the next apiVersion. - maxLength: 256 - type: string name: description: name of the variable. maxLength: 256 diff --git a/internal/controllers/topology/cluster/patches/engine.go b/internal/controllers/topology/cluster/patches/engine.go index 8a0870a878a2..12b83221417c 100644 --- a/internal/controllers/topology/cluster/patches/engine.go +++ b/internal/controllers/topology/cluster/patches/engine.go @@ -598,7 +598,7 @@ func updateDesiredState(ctx context.Context, req *runtimehooksv1.GeneratePatches return nil } -// definitionsForPatch returns a set which of variables in a ClusterClass defined by "definitionFrom". +// definitionsForPatch returns a set of variables in a ClusterClass defined by "definitionFrom". func definitionsForPatch(blueprint *scope.ClusterBlueprint, definitionFrom string) map[string]bool { variableDefinitionsForPatch := make(map[string]bool) for _, definitionsWithName := range blueprint.ClusterClass.Status.Variables { diff --git a/internal/topology/variables/cluster_variable_defaulting_test.go b/internal/topology/variables/cluster_variable_defaulting_test.go index 0ed5a1db2360..514d65581ecb 100644 --- a/internal/topology/variables/cluster_variable_defaulting_test.go +++ b/internal/topology/variables/cluster_variable_defaulting_test.go @@ -492,54 +492,6 @@ func Test_DefaultClusterVariables(t *testing.T) { }, createVariables: true, }, - { - name: "Error if a value is set with non-empty definitionFrom.", - wantErrs: []validationMatch{ - invalid("Invalid value: \"2\": variable \"cpu\" has DefinitionFrom set. DefinitionFrom is deprecated, must not be set anymore and is going to be removed in the next apiVersion", - "spec.topology.variables[cpu]"), - invalid("Invalid value: \"2\": variable \"cpu\" is set more than once", - "spec.topology.variables[cpu]"), - }, - definitions: []clusterv1.ClusterClassStatusVariable{ - { - Name: "cpu", - Definitions: []clusterv1.ClusterClassStatusVariableDefinition{ - { - Schema: clusterv1.VariableSchema{ - OpenAPIV3Schema: clusterv1.JSONSchemaProps{ - Type: "integer", - }, - }, - From: "somepatch", - }, - { - Schema: clusterv1.VariableSchema{ - OpenAPIV3Schema: clusterv1.JSONSchemaProps{ - Type: "integer", - }, - }, - From: clusterv1.VariableDefinitionFromInline, - }, - }, - }, - }, - values: []clusterv1.ClusterVariable{ - { - Name: "cpu", - Value: apiextensionsv1.JSON{ - Raw: []byte(`1`), - }, - }, - { - Name: "cpu", - Value: apiextensionsv1.JSON{ - Raw: []byte(`2`), - }, - // Non-empty definitionFrom is not valid. - DefinitionFrom: "somepatch", - }, - }, - }, { name: "Error if a value is set twice.", wantErrs: []validationMatch{ diff --git a/internal/topology/variables/cluster_variable_validation_test.go b/internal/topology/variables/cluster_variable_validation_test.go index 95b9e48d2504..86abc6d41d47 100644 --- a/internal/topology/variables/cluster_variable_validation_test.go +++ b/internal/topology/variables/cluster_variable_validation_test.go @@ -265,39 +265,6 @@ func Test_ValidateClusterVariables(t *testing.T) { }, validateRequired: true, }, - { - name: "Fail if DefinitionFrom not empty.", - wantErrs: []validationMatch{ - invalid("Invalid value: \"1\": variable \"cpu\" has DefinitionFrom set. DefinitionFrom is deprecated, must not be set anymore and is going to be removed in the next apiVersion", - "spec.topology.variables[cpu]"), - }, - definitions: []clusterv1.ClusterClassStatusVariable{ - { - Name: "cpu", - Definitions: []clusterv1.ClusterClassStatusVariableDefinition{ - { - From: clusterv1.VariableDefinitionFromInline, - Schema: clusterv1.VariableSchema{ - OpenAPIV3Schema: clusterv1.JSONSchemaProps{ - Type: "integer", - }, - }, - }, - }, - }, - }, - values: []clusterv1.ClusterVariable{ - { - Name: "cpu", - Value: apiextensionsv1.JSON{ - Raw: []byte(`1`), - }, - // Non-empty definitionFrom is not valid. - DefinitionFrom: clusterv1.VariableDefinitionFromInline, - }, - }, - validateRequired: true, - }, { name: "Fail if a value is set twice.", wantErrs: []validationMatch{ @@ -335,55 +302,6 @@ func Test_ValidateClusterVariables(t *testing.T) { }, validateRequired: true, }, - { - name: "Fail if DefinitionFrom not empty and value is set twice.", - wantErrs: []validationMatch{ - invalid("Invalid value: \"2\": variable \"cpu\" has DefinitionFrom set. DefinitionFrom is deprecated, must not be set anymore and is going to be removed in the next apiVersion", - "spec.topology.variables[cpu]"), - invalid("Invalid value: \"2\": variable \"cpu\" is set more than once", - "spec.topology.variables[cpu]"), - }, - definitions: []clusterv1.ClusterClassStatusVariable{ - { - Name: "cpu", - Definitions: []clusterv1.ClusterClassStatusVariableDefinition{ - { - Schema: clusterv1.VariableSchema{ - OpenAPIV3Schema: clusterv1.JSONSchemaProps{ - Type: "integer", - }, - }, - From: "somepatch", - }, - { - Schema: clusterv1.VariableSchema{ - OpenAPIV3Schema: clusterv1.JSONSchemaProps{ - Type: "integer", - }, - }, - From: clusterv1.VariableDefinitionFromInline, - }, - }, - }, - }, - values: []clusterv1.ClusterVariable{ - { - Name: "cpu", - Value: apiextensionsv1.JSON{ - Raw: []byte(`1`), - }, - }, - { - Name: "cpu", - Value: apiextensionsv1.JSON{ - Raw: []byte(`2`), - }, - // Non-empty definitionFrom is not valid. - DefinitionFrom: "somepatch", - }, - }, - validateRequired: true, - }, { name: "Fail when value invalid by their definition schema.", wantErrs: []validationMatch{ @@ -2498,38 +2416,6 @@ func Test_ValidateMachineVariables(t *testing.T) { }, }, }, - { - name: "Fail if value DefinitionFrom is not empty", - wantErrs: []validationMatch{ - invalid("Invalid value: \"1\": variable \"cpu\" has DefinitionFrom set. DefinitionFrom is deprecated, must not be set anymore and is going to be removed in the next apiVersion", - "spec.topology.workers.machineDeployments[mdTopologyName].variables.overrides[cpu]"), - }, - definitions: []clusterv1.ClusterClassStatusVariable{ - { - Name: "cpu", - Definitions: []clusterv1.ClusterClassStatusVariableDefinition{ - { - From: clusterv1.VariableDefinitionFromInline, - Schema: clusterv1.VariableSchema{ - OpenAPIV3Schema: clusterv1.JSONSchemaProps{ - Type: "string", - }, - }, - }, - }, - }, - }, - values: []clusterv1.ClusterVariable{ - { - Name: "cpu", - Value: apiextensionsv1.JSON{ - Raw: []byte(`1`), - }, - // Non-empty definitionFrom is not valid. - DefinitionFrom: "non-existent-patch", - }, - }, - }, { name: "Fail when value invalid by their definition schema.", wantErrs: []validationMatch{ diff --git a/internal/topology/variables/utils.go b/internal/topology/variables/utils.go index 5afc81554e7b..bfaf09a36162 100644 --- a/internal/topology/variables/utils.go +++ b/internal/topology/variables/utils.go @@ -28,18 +28,11 @@ import ( ) // newValuesIndex returns a map of ClusterVariable per name. -// This function validates that: -// - DefinitionFrom is not set -// - variables are not defined more than once. +// This function validates that variables are not defined more than once. func newValuesIndex(fldPath *field.Path, values []clusterv1.ClusterVariable) (map[string]*clusterv1.ClusterVariable, field.ErrorList) { valuesMap := map[string]*clusterv1.ClusterVariable{} errs := field.ErrorList{} for _, value := range values { - // Check that the variable has DefinitionFrom not set. - if value.DefinitionFrom != "" { //nolint:staticcheck // Intentionally using the deprecated field here to check that it is not set. - errs = append(errs, field.Invalid(fldPath.Key(value.Name), string(value.Value.Raw), fmt.Sprintf("variable %q has DefinitionFrom set. DefinitionFrom is deprecated, must not be set anymore and is going to be removed in the next apiVersion", value.Name))) - } - // Check that the variable has not been defined more than once. if _, ok := valuesMap[value.Name]; ok { errs = append(errs, field.Invalid(fldPath.Key(value.Name), string(value.Value.Raw), fmt.Sprintf("variable %q is set more than once", value.Name))) diff --git a/internal/webhooks/cluster.go b/internal/webhooks/cluster.go index d05ff97f2ff1..b038b28eb061 100644 --- a/internal/webhooks/cluster.go +++ b/internal/webhooks/cluster.go @@ -333,9 +333,6 @@ func (webhook *Cluster) validateTopology(ctx context.Context, oldCluster, newClu // metadata in topology should be valid allErrs = append(allErrs, validateTopologyMetadata(newCluster.Spec.Topology, fldPath)...) - // ensure deprecationFrom is not set - allErrs = append(allErrs, validateTopologyDefinitionFrom(newCluster.Spec.Topology, fldPath)...) - // upgrade concurrency should be a numeric value. if concurrency, ok := newCluster.Annotations[clusterv1.ClusterTopologyUpgradeConcurrencyAnnotation]; ok { concurrencyAnnotationField := field.NewPath("metadata", "annotations", clusterv1.ClusterTopologyUpgradeConcurrencyAnnotation) @@ -1025,62 +1022,6 @@ func validateTopologyMetadata(topology *clusterv1.Topology, fldPath *field.Path) return allErrs } -func validateTopologyDefinitionFrom(topology *clusterv1.Topology, fldPath *field.Path) field.ErrorList { - var allErrs field.ErrorList - for _, variable := range topology.Variables { - if variable.DefinitionFrom != "" { //nolint:staticcheck // Intentionally using the deprecated field here to check that it is not set. - allErrs = append(allErrs, field.Invalid( - fldPath.Child("variables").Key(variable.Name), - string(variable.Value.Raw), - fmt.Sprintf("variable %q has DefinitionFrom set", variable.Name)), - ) - } - } - - if topology.ControlPlane.Variables != nil { - for _, variable := range topology.ControlPlane.Variables.Overrides { - if variable.DefinitionFrom != "" { //nolint:staticcheck // Intentionally using the deprecated field here to check that it is not set. - allErrs = append(allErrs, field.Invalid( - fldPath.Child("controlPlane", "variables", "overrides").Key(variable.Name), - string(variable.Value.Raw), - fmt.Sprintf("variable %q has DefinitionFrom set", variable.Name)), - ) - } - } - } - - if topology.Workers != nil { - for _, md := range topology.Workers.MachineDeployments { - if md.Variables != nil { - for _, variable := range md.Variables.Overrides { - if variable.DefinitionFrom != "" { //nolint:staticcheck // Intentionally using the deprecated field here to check that it is not set. - allErrs = append(allErrs, field.Invalid( - fldPath.Child("workers", "machineDeployments").Key(md.Name).Child("variables", "overrides").Key(variable.Name), - string(variable.Value.Raw), - fmt.Sprintf("variable %q has DefinitionFrom set", variable.Name)), - ) - } - } - } - } - for _, mp := range topology.Workers.MachinePools { - if mp.Variables != nil { - for _, variable := range mp.Variables.Overrides { - if variable.DefinitionFrom != "" { //nolint:staticcheck // Intentionally using the deprecated field here to check that it is not set. - allErrs = append(allErrs, field.Invalid( - fldPath.Child("workers", "machinePools").Key(mp.Name).Child("variables", "overrides").Key(variable.Name), - string(variable.Value.Raw), - fmt.Sprintf("variable %q has DefinitionFrom set", variable.Name)), - ) - } - } - } - } - } - - return allErrs -} - // validateAutoscalerAnnotationsForCluster iterates the MachineDeploymentsTopology objects under Workers and ensures the replicas // field and min/max annotations for autoscaler are not set at the same time. Optionally it also checks if a given ClusterClass has // the annotations that may apply to this Cluster. diff --git a/internal/webhooks/cluster_test.go b/internal/webhooks/cluster_test.go index 67d7006f6ad4..77c8e9f31e0e 100644 --- a/internal/webhooks/cluster_test.go +++ b/internal/webhooks/cluster_test.go @@ -2114,74 +2114,6 @@ func TestClusterTopologyValidation(t *testing.T) { }).WithVersion("v1.18.1").Build(), }, }, - { - name: "should return error if DefinitionFrom is set on a Cluster variable", - expectErr: true, - in: builder.Cluster("fooboo", "cluster1"). - WithTopology(builder.ClusterTopology(). - WithClass("foo"). - WithVersion("v1.30.0"). - WithVariables(clusterv1.ClusterVariable{ - Name: "variable-1", - DefinitionFrom: "patch-1", - Value: apiextensionsv1.JSON{}, - }). - Build()). - Build(), - }, - { - name: "should return error if DefinitionFrom is set on a control plane variable override", - expectErr: true, - in: builder.Cluster("fooboo", "cluster1"). - WithTopology(builder.ClusterTopology(). - WithClass("foo"). - WithVersion("v1.30.0"). - WithControlPlaneVariables(clusterv1.ClusterVariable{ - Name: "variable-1", - DefinitionFrom: "patch-1", - Value: apiextensionsv1.JSON{}, - }). - Build()). - Build(), - }, - { - name: "should return error if DefinitionFrom is set on a MD variable override", - expectErr: true, - in: builder.Cluster("fooboo", "cluster1"). - WithTopology(builder.ClusterTopology(). - WithClass("foo"). - WithVersion("v1.30.0"). - WithMachineDeployment( - builder.MachineDeploymentTopology("md1"). - WithClass("bb"). - WithVariables(clusterv1.ClusterVariable{ - Name: "variable-1", - DefinitionFrom: "patch-1", - Value: apiextensionsv1.JSON{}, - }). - Build()). - Build()). - Build(), - }, - { - name: "should return error if DefinitionFrom is set on a MP variable override", - expectErr: true, - in: builder.Cluster("fooboo", "cluster1"). - WithTopology(builder.ClusterTopology(). - WithClass("foo"). - WithVersion("v1.30.0"). - WithMachinePool( - builder.MachinePoolTopology("mp1"). - WithClass("bb"). - WithVariables(clusterv1.ClusterVariable{ - Name: "variable-1", - DefinitionFrom: "patch-1", - Value: apiextensionsv1.JSON{}, - }). - Build()). - Build()). - Build(), - }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {