diff --git a/api/v1beta1/cluster_types.go b/api/v1beta1/cluster_types.go index 12c258f22a0a..53cb96271c76 100644 --- a/api/v1beta1/cluster_types.go +++ b/api/v1beta1/cluster_types.go @@ -142,6 +142,10 @@ type ControlPlaneTopology struct { // Defaults to 10 seconds. // +optional NodeDeletionTimeout *metav1.Duration `json:"nodeDeletionTimeout,omitempty"` + + // Variables can be used to customize the ControlPlane through patches. + // +optional + Variables *ControlPlaneVariables `json:"variables,omitempty"` } // WorkersTopology represents the different sets of worker nodes in the cluster. @@ -327,6 +331,13 @@ type ClusterVariable struct { Value apiextensionsv1.JSON `json:"value"` } +// ControlPlaneVariables can be used to provide variables for the ControlPlane. +type ControlPlaneVariables struct { + // Overrides can be used to override Cluster level variables. + // +optional + Overrides []ClusterVariable `json:"overrides,omitempty"` +} + // MachineDeploymentVariables can be used to provide variables for a specific MachineDeployment. type MachineDeploymentVariables struct { // Overrides can be used to override Cluster level variables. diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index cd8e2982e3d9..695ac1cea34a 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -619,6 +619,11 @@ func (in *ControlPlaneTopology) DeepCopyInto(out *ControlPlaneTopology) { *out = new(metav1.Duration) **out = **in } + if in.Variables != nil { + in, out := &in.Variables, &out.Variables + *out = new(ControlPlaneVariables) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ControlPlaneTopology. @@ -631,6 +636,28 @@ func (in *ControlPlaneTopology) DeepCopy() *ControlPlaneTopology { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ControlPlaneVariables) DeepCopyInto(out *ControlPlaneVariables) { + *out = *in + if in.Overrides != nil { + in, out := &in.Overrides, &out.Overrides + *out = make([]ClusterVariable, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ControlPlaneVariables. +func (in *ControlPlaneVariables) DeepCopy() *ControlPlaneVariables { + if in == nil { + return nil + } + out := new(ControlPlaneVariables) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ExternalPatchDefinition) DeepCopyInto(out *ExternalPatchDefinition) { *out = *in diff --git a/api/v1beta1/zz_generated.openapi.go b/api/v1beta1/zz_generated.openapi.go index 26d1d17859fd..28cca7cb69a6 100644 --- a/api/v1beta1/zz_generated.openapi.go +++ b/api/v1beta1/zz_generated.openapi.go @@ -49,6 +49,7 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA "sigs.k8s.io/cluster-api/api/v1beta1.ControlPlaneClass": schema_sigsk8sio_cluster_api_api_v1beta1_ControlPlaneClass(ref), "sigs.k8s.io/cluster-api/api/v1beta1.ControlPlaneClassNamingStrategy": schema_sigsk8sio_cluster_api_api_v1beta1_ControlPlaneClassNamingStrategy(ref), "sigs.k8s.io/cluster-api/api/v1beta1.ControlPlaneTopology": schema_sigsk8sio_cluster_api_api_v1beta1_ControlPlaneTopology(ref), + "sigs.k8s.io/cluster-api/api/v1beta1.ControlPlaneVariables": schema_sigsk8sio_cluster_api_api_v1beta1_ControlPlaneVariables(ref), "sigs.k8s.io/cluster-api/api/v1beta1.ExternalPatchDefinition": schema_sigsk8sio_cluster_api_api_v1beta1_ExternalPatchDefinition(ref), "sigs.k8s.io/cluster-api/api/v1beta1.FailureDomainSpec": schema_sigsk8sio_cluster_api_api_v1beta1_FailureDomainSpec(ref), "sigs.k8s.io/cluster-api/api/v1beta1.JSONPatch": schema_sigsk8sio_cluster_api_api_v1beta1_JSONPatch(ref), @@ -1115,11 +1116,46 @@ func schema_sigsk8sio_cluster_api_api_v1beta1_ControlPlaneTopology(ref common.Re Ref: ref("k8s.io/apimachinery/pkg/apis/meta/v1.Duration"), }, }, + "variables": { + SchemaProps: spec.SchemaProps{ + Description: "Variables can be used to customize the ControlPlane through patches.", + Ref: ref("sigs.k8s.io/cluster-api/api/v1beta1.ControlPlaneVariables"), + }, + }, }, }, }, Dependencies: []string{ - "k8s.io/apimachinery/pkg/apis/meta/v1.Duration", "sigs.k8s.io/cluster-api/api/v1beta1.MachineHealthCheckTopology", "sigs.k8s.io/cluster-api/api/v1beta1.ObjectMeta"}, + "k8s.io/apimachinery/pkg/apis/meta/v1.Duration", "sigs.k8s.io/cluster-api/api/v1beta1.ControlPlaneVariables", "sigs.k8s.io/cluster-api/api/v1beta1.MachineHealthCheckTopology", "sigs.k8s.io/cluster-api/api/v1beta1.ObjectMeta"}, + } +} + +func schema_sigsk8sio_cluster_api_api_v1beta1_ControlPlaneVariables(ref common.ReferenceCallback) common.OpenAPIDefinition { + return common.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Description: "ControlPlaneVariables can be used to provide variables for the ControlPlane.", + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "overrides": { + SchemaProps: spec.SchemaProps{ + Description: "Overrides can be used to override Cluster level variables.", + Type: []string{"array"}, + Items: &spec.SchemaOrArray{ + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Default: map[string]interface{}{}, + Ref: ref("sigs.k8s.io/cluster-api/api/v1beta1.ClusterVariable"), + }, + }, + }, + }, + }, + }, + }, + }, + Dependencies: []string{ + "sigs.k8s.io/cluster-api/api/v1beta1.ClusterVariable"}, } } diff --git a/config/crd/bases/cluster.x-k8s.io_clusters.yaml b/config/crd/bases/cluster.x-k8s.io_clusters.yaml index 1e04b861cad6..ed61a3d73a00 100644 --- a/config/crd/bases/cluster.x-k8s.io_clusters.yaml +++ b/config/crd/bases/cluster.x-k8s.io_clusters.yaml @@ -1084,6 +1084,44 @@ spec: When specified against a control plane provider that lacks support for this field, this value will be ignored. format: int32 type: integer + variables: + description: Variables can be used to customize the ControlPlane + through patches. + properties: + overrides: + description: Overrides can be used to override Cluster + level variables. + items: + description: |- + 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. DefinitionFrom is `inline` when the + definition is from the ClusterClass `.spec.variables` or the name of a patch defined in the ClusterClass + `.spec.patches` where the patch is external and provides external variables. + This field is mandatory if the variable has `DefinitionsConflict: true` in ClusterClass `status.variables[]` + type: string + name: + description: Name of the variable. + type: string + value: + 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 + x-kubernetes-preserve-unknown-fields: true + required: + - name + - value + type: object + type: array + type: object type: object rolloutAfter: description: |- diff --git a/internal/apis/core/v1alpha4/conversion.go b/internal/apis/core/v1alpha4/conversion.go index 368dff1b1d94..3b58a00e7ad5 100644 --- a/internal/apis/core/v1alpha4/conversion.go +++ b/internal/apis/core/v1alpha4/conversion.go @@ -42,6 +42,7 @@ func (src *Cluster) ConvertTo(dstRaw conversion.Hub) error { dst.Spec.Topology = &clusterv1.Topology{} } dst.Spec.Topology.Variables = restored.Spec.Topology.Variables + dst.Spec.Topology.ControlPlane.Variables = restored.Spec.Topology.ControlPlane.Variables if restored.Spec.Topology.ControlPlane.MachineHealthCheck != nil { dst.Spec.Topology.ControlPlane.MachineHealthCheck = restored.Spec.Topology.ControlPlane.MachineHealthCheck diff --git a/internal/apis/core/v1alpha4/zz_generated.conversion.go b/internal/apis/core/v1alpha4/zz_generated.conversion.go index e89720cc15fc..094278ba6eaa 100644 --- a/internal/apis/core/v1alpha4/zz_generated.conversion.go +++ b/internal/apis/core/v1alpha4/zz_generated.conversion.go @@ -892,6 +892,7 @@ func autoConvert_v1beta1_ControlPlaneTopology_To_v1alpha4_ControlPlaneTopology(i // WARNING: in.NodeDrainTimeout requires manual conversion: does not exist in peer-type // WARNING: in.NodeVolumeDetachTimeout requires manual conversion: does not exist in peer-type // WARNING: in.NodeDeletionTimeout requires manual conversion: does not exist in peer-type + // WARNING: in.Variables requires manual conversion: does not exist in peer-type return nil } diff --git a/internal/controllers/topology/cluster/cluster_controller_test.go b/internal/controllers/topology/cluster/cluster_controller_test.go index 8d14c8592ae7..d812172a62fa 100644 --- a/internal/controllers/topology/cluster/cluster_controller_test.go +++ b/internal/controllers/topology/cluster/cluster_controller_test.go @@ -1371,6 +1371,9 @@ func TestReconciler_DefaultCluster(t *testing.T) { WithVariables( clusterv1.ClusterVariable{Name: "location", Value: apiextensionsv1.JSON{Raw: []byte(`"us-west"`)}}, clusterv1.ClusterVariable{Name: "httpProxy", Value: apiextensionsv1.JSON{Raw: []byte(`{"enabled":true}`)}}). + WithControlPlaneVariables( + clusterv1.ClusterVariable{Name: "location", Value: apiextensionsv1.JSON{Raw: []byte(`"us-west"`)}}, + clusterv1.ClusterVariable{Name: "httpProxy", Value: apiextensionsv1.JSON{Raw: []byte(`{"enabled":true}`)}}). WithMachineDeployment(mdTopologyBase.DeepCopy(). WithVariables(clusterv1.ClusterVariable{ Name: "httpProxy", @@ -1388,6 +1391,9 @@ func TestReconciler_DefaultCluster(t *testing.T) { WithVariables( clusterv1.ClusterVariable{Name: "location", Value: apiextensionsv1.JSON{Raw: []byte(`"us-west"`)}}, clusterv1.ClusterVariable{Name: "httpProxy", Value: apiextensionsv1.JSON{Raw: []byte(`{"enabled":true,"url":"http://localhost:3128"}`)}}). + WithControlPlaneVariables( + clusterv1.ClusterVariable{Name: "location", Value: apiextensionsv1.JSON{Raw: []byte(`"us-west"`)}}, + clusterv1.ClusterVariable{Name: "httpProxy", Value: apiextensionsv1.JSON{Raw: []byte(`{"enabled":true,"url":"http://localhost:3128"}`)}}). WithMachineDeployment( mdTopologyBase.DeepCopy().WithVariables( clusterv1.ClusterVariable{ diff --git a/internal/controllers/topology/cluster/patches/engine.go b/internal/controllers/topology/cluster/patches/engine.go index 0257bf824839..963b220cc8d9 100644 --- a/internal/controllers/topology/cluster/patches/engine.go +++ b/internal/controllers/topology/cluster/patches/engine.go @@ -165,7 +165,7 @@ func addVariablesForPatch(blueprint *scope.ClusterBlueprint, desired *scope.Clus req.Variables = globalVariables // Calculate the Control Plane variables. - controlPlaneVariables, err := variables.ControlPlane(&blueprint.Topology.ControlPlane, desired.ControlPlane.Object, desired.ControlPlane.InfrastructureMachineTemplate) + controlPlaneVariables, err := variables.ControlPlane(&blueprint.Topology.ControlPlane, desired.ControlPlane.Object, desired.ControlPlane.InfrastructureMachineTemplate, definitionFrom, patchVariableDefinitions) if err != nil { return errors.Wrapf(err, "failed to calculate ControlPlane variables") } diff --git a/internal/controllers/topology/cluster/patches/engine_test.go b/internal/controllers/topology/cluster/patches/engine_test.go index 0946a0fa9b2a..a288df6dc36c 100644 --- a/internal/controllers/topology/cluster/patches/engine_test.go +++ b/internal/controllers/topology/cluster/patches/engine_test.go @@ -701,7 +701,7 @@ func TestApply(t *testing.T) { name: "Should correctly apply variables for a given patch definitionFrom", varDefinitions: []clusterv1.ClusterClassStatusVariable{ { - Name: "default-worker-infra", + Name: "controlPlaneVariable", Definitions: []clusterv1.ClusterClassStatusVariableDefinition{ { From: "inline", @@ -709,7 +709,15 @@ func TestApply(t *testing.T) { }, }, { - Name: "default-mp-worker-infra", + Name: "defaultMDWorkerVariable", + Definitions: []clusterv1.ClusterClassStatusVariableDefinition{ + { + From: "inline", + }, + }, + }, + { + Name: "defaultMPWorkerVariable", Definitions: []clusterv1.ClusterClassStatusVariableDefinition{ { From: "inline", @@ -751,6 +759,42 @@ func TestApply(t *testing.T) { }, }, }, + { + Selector: clusterv1.PatchSelector{ + APIVersion: builder.ControlPlaneGroupVersion.String(), + Kind: builder.GenericControlPlaneTemplateKind, + MatchResources: clusterv1.PatchSelectorMatch{ + ControlPlane: true, + }, + }, + JSONPatches: []clusterv1.JSONPatch{ + { + Op: "add", + Path: "/spec/template/spec/controlPlaneField", + ValueFrom: &clusterv1.JSONPatchValue{ + Variable: ptr.To("controlPlaneVariable"), + }, + }, + }, + }, + { + Selector: clusterv1.PatchSelector{ + APIVersion: builder.InfrastructureGroupVersion.String(), + Kind: builder.GenericInfrastructureMachineTemplateKind, + MatchResources: clusterv1.PatchSelectorMatch{ + ControlPlane: true, + }, + }, + JSONPatches: []clusterv1.JSONPatch{ + { + Op: "add", + Path: "/spec/template/spec/controlPlaneInfraMachineTemplateField", + ValueFrom: &clusterv1.JSONPatchValue{ + Variable: ptr.To("controlPlaneVariable"), + }, + }, + }, + }, { Selector: clusterv1.PatchSelector{ APIVersion: builder.BootstrapGroupVersion.String(), @@ -766,7 +810,7 @@ func TestApply(t *testing.T) { Op: "add", Path: "/spec/template/spec/resource", ValueFrom: &clusterv1.JSONPatchValue{ - Variable: ptr.To("default-worker-infra"), + Variable: ptr.To("defaultMDWorkerVariable"), }, }, }, @@ -786,7 +830,7 @@ func TestApply(t *testing.T) { Op: "add", Path: "/spec/template/spec/resource", ValueFrom: &clusterv1.JSONPatchValue{ - Variable: ptr.To("default-worker-infra"), + Variable: ptr.To("defaultMDWorkerVariable"), }, }, }, @@ -806,7 +850,7 @@ func TestApply(t *testing.T) { Op: "add", Path: "/spec/template/spec/resource", ValueFrom: &clusterv1.JSONPatchValue{ - Variable: ptr.To("default-mp-worker-infra"), + Variable: ptr.To("defaultMPWorkerVariable"), }, }, }, @@ -826,7 +870,7 @@ func TestApply(t *testing.T) { Op: "add", Path: "/spec/template/spec/resource", ValueFrom: &clusterv1.JSONPatchValue{ - Variable: ptr.To("default-mp-worker-infra"), + Variable: ptr.To("defaultMPWorkerVariable"), }, }, }, @@ -838,21 +882,27 @@ func TestApply(t *testing.T) { infrastructureCluster: map[string]interface{}{ "spec.resource": "value99", }, + controlPlane: map[string]interface{}{ + "spec.controlPlaneField": "control-plane-override-value", + }, + controlPlaneInfrastructureMachineTemplate: map[string]interface{}{ + "spec.template.spec.controlPlaneInfraMachineTemplateField": "control-plane-override-value", + }, machineDeploymentInfrastructureMachineTemplate: map[string]map[string]interface{}{ - "default-worker-topo1": {"spec.template.spec.resource": "value1"}, - "default-worker-topo2": {"spec.template.spec.resource": "default-worker-topo2"}, + "default-worker-topo1": {"spec.template.spec.resource": "default-worker-topo1-override-value"}, + "default-worker-topo2": {"spec.template.spec.resource": "default-md-cluster-wide-value"}, }, machineDeploymentBootstrapTemplate: map[string]map[string]interface{}{ - "default-worker-topo1": {"spec.template.spec.resource": "value1"}, - "default-worker-topo2": {"spec.template.spec.resource": "default-worker-topo2"}, + "default-worker-topo1": {"spec.template.spec.resource": "default-worker-topo1-override-value"}, + "default-worker-topo2": {"spec.template.spec.resource": "default-md-cluster-wide-value"}, }, machinePoolInfrastructureMachinePool: map[string]map[string]interface{}{ - "default-mp-worker-topo1": {"spec.resource": "value2"}, - "default-mp-worker-topo2": {"spec.resource": "default-mp-worker-topo2"}, + "default-mp-worker-topo1": {"spec.resource": "default-mp-worker-topo1-override-value"}, + "default-mp-worker-topo2": {"spec.resource": "default-mp-cluster-wide-value"}, }, machinePoolBootstrapConfig: map[string]map[string]interface{}{ - "default-mp-worker-topo1": {"spec.resource": "value2"}, - "default-mp-worker-topo2": {"spec.resource": "default-mp-worker-topo2"}, + "default-mp-worker-topo1": {"spec.resource": "default-mp-worker-topo1-override-value"}, + "default-mp-worker-topo2": {"spec.resource": "default-mp-cluster-wide-value"}, }, }, }, @@ -1043,6 +1093,15 @@ func setupTestObjects() (*scope.ClusterBlueprint, *scope.ClusterState) { Class: clusterClass.Name, ControlPlane: clusterv1.ControlPlaneTopology{ Replicas: ptr.To[int32](3), + Variables: &clusterv1.ControlPlaneVariables{ + Overrides: []clusterv1.ClusterVariable{ + { + Name: "controlPlaneVariable", + DefinitionFrom: "inline", + Value: apiextensionsv1.JSON{Raw: []byte(`"control-plane-override-value"`)}, + }, + }, + }, }, Variables: []clusterv1.ClusterVariable{ { @@ -1057,15 +1116,21 @@ func setupTestObjects() (*scope.ClusterBlueprint, *scope.ClusterState) { DefinitionFrom: "not-used-patch", }, { - Name: "default-worker-infra", + Name: "controlPlaneVariable", + // This value should be overwritten for the control plane. + Value: apiextensionsv1.JSON{Raw: []byte(`"control-plane-cluster-wide-value"`)}, + DefinitionFrom: "inline", + }, + { + Name: "defaultMDWorkerVariable", // This value should be overwritten for the default-worker-topo1 MachineDeployment. - Value: apiextensionsv1.JSON{Raw: []byte(`"default-worker-topo2"`)}, + Value: apiextensionsv1.JSON{Raw: []byte(`"default-md-cluster-wide-value"`)}, DefinitionFrom: "inline", }, { - Name: "default-mp-worker-infra", + Name: "defaultMPWorkerVariable", // This value should be overwritten for the default-mp-worker-topo1 MachinePool. - Value: apiextensionsv1.JSON{Raw: []byte(`"default-mp-worker-topo2"`)}, + Value: apiextensionsv1.JSON{Raw: []byte(`"default-mp-cluster-wide-value"`)}, DefinitionFrom: "inline", }, }, @@ -1078,9 +1143,9 @@ func setupTestObjects() (*scope.ClusterBlueprint, *scope.ClusterState) { Variables: &clusterv1.MachineDeploymentVariables{ Overrides: []clusterv1.ClusterVariable{ { - Name: "default-worker-infra", + Name: "defaultMDWorkerVariable", DefinitionFrom: "inline", - Value: apiextensionsv1.JSON{Raw: []byte(`"value1"`)}, + Value: apiextensionsv1.JSON{Raw: []byte(`"default-worker-topo1-override-value"`)}, }, }, }, @@ -1100,9 +1165,9 @@ func setupTestObjects() (*scope.ClusterBlueprint, *scope.ClusterState) { Variables: &clusterv1.MachinePoolVariables{ Overrides: []clusterv1.ClusterVariable{ { - Name: "default-mp-worker-infra", + Name: "defaultMPWorkerVariable", DefinitionFrom: "inline", - Value: apiextensionsv1.JSON{Raw: []byte(`"value2"`)}, + Value: apiextensionsv1.JSON{Raw: []byte(`"default-mp-worker-topo1-override-value"`)}, }, }, }, diff --git a/internal/controllers/topology/cluster/patches/variables/variables.go b/internal/controllers/topology/cluster/patches/variables/variables.go index 9887f0739ef5..e47380e12d92 100644 --- a/internal/controllers/topology/cluster/patches/variables/variables.go +++ b/internal/controllers/topology/cluster/patches/variables/variables.go @@ -94,9 +94,22 @@ func Global(clusterTopology *clusterv1.Topology, cluster *clusterv1.Cluster, def } // ControlPlane returns variables that apply to templates belonging to the ControlPlane. -func ControlPlane(cpTopology *clusterv1.ControlPlaneTopology, cp, cpInfrastructureMachineTemplate *unstructured.Unstructured) ([]runtimehooksv1.Variable, error) { +func ControlPlane(cpTopology *clusterv1.ControlPlaneTopology, cp, cpInfrastructureMachineTemplate *unstructured.Unstructured, definitionFrom string, patchVariableDefinitions map[string]bool) ([]runtimehooksv1.Variable, error) { variables := []runtimehooksv1.Variable{} + // Add variables overrides for the ControlPlane. + if cpTopology.Variables != nil { + for _, variable := range cpTopology.Variables.Overrides { + // Add the variable if it is defined for the current patch or it is defined for all the patches. + if variable.DefinitionFrom == emptyDefinitionFrom || variable.DefinitionFrom == definitionFrom { + // Add the variable if it has a definition from this patch in the ClusterClass. + if _, ok := patchVariableDefinitions[variable.Name]; ok { + variables = append(variables, runtimehooksv1.Variable{Name: variable.Name, Value: variable.Value}) + } + } + } + } + // Construct builtin variable. builtin := runtimehooksv1.Builtins{ ControlPlane: &runtimehooksv1.ControlPlaneBuiltins{ diff --git a/internal/controllers/topology/cluster/patches/variables/variables_test.go b/internal/controllers/topology/cluster/patches/variables/variables_test.go index 52ff8b4fee77..f564df166393 100644 --- a/internal/controllers/topology/cluster/patches/variables/variables_test.go +++ b/internal/controllers/topology/cluster/patches/variables/variables_test.go @@ -405,20 +405,44 @@ func TestControlPlane(t *testing.T) { tests := []struct { name string controlPlaneTopology *clusterv1.ControlPlaneTopology + forPatch string + variableDefinitionsForPatch map[string]bool controlPlane *unstructured.Unstructured controlPlaneInfrastructureMachineTemplate *unstructured.Unstructured want []runtimehooksv1.Variable }{ { - name: "Should calculate ControlPlane variables", + name: "Should calculate ControlPlane variables", + variableDefinitionsForPatch: map[string]bool{"location": true, "cpu": true}, + forPatch: "patch1", controlPlaneTopology: &clusterv1.ControlPlaneTopology{ Replicas: ptr.To[int32](3), + Variables: &clusterv1.ControlPlaneVariables{ + Overrides: []clusterv1.ClusterVariable{ + { + Name: "location", + Value: toJSON("\"us-central\""), + }, + { + Name: "cpu", + Value: toJSON("8"), + }, + }, + }, }, controlPlane: builder.ControlPlane(metav1.NamespaceDefault, "controlPlane1"). WithReplicas(3). WithVersion("v1.21.1"). Build(), want: []runtimehooksv1.Variable{ + { + Name: "location", + Value: toJSON("\"us-central\""), + }, + { + Name: "cpu", + Value: toJSON("8"), + }, { Name: runtimehooksv1.BuiltinsName, Value: toJSONCompact(`{ @@ -431,12 +455,116 @@ func TestControlPlane(t *testing.T) { }, }, { - name: "Should calculate ControlPlane variables, replicas not set", - controlPlaneTopology: &clusterv1.ControlPlaneTopology{}, + name: "Should calculate ControlPlane variables for a given patch name", + variableDefinitionsForPatch: map[string]bool{"location": true, "cpu": true}, + forPatch: "patch1", + controlPlaneTopology: &clusterv1.ControlPlaneTopology{ + Replicas: ptr.To[int32](3), + Variables: &clusterv1.ControlPlaneVariables{ + Overrides: []clusterv1.ClusterVariable{ + { + Name: "location", + Value: toJSON("\"us-central\""), + DefinitionFrom: "patch1", + }, + { + Name: "location", + Value: toJSON("\"us-east\""), + // This variable should be excluded because it is defined for a different patch. + DefinitionFrom: "anotherPatch", + }, + + { + Name: "http-proxy", + Value: toJSON("\"internal.proxy.com\""), + // This variable should be excluded because it is not in variableDefinitionsForPatch. + DefinitionFrom: "", + }, + { + Name: "cpu", + Value: toJSON("8"), + // This variable should be included because it is defined for all patches. + }, + }, + }, + }, controlPlane: builder.ControlPlane(metav1.NamespaceDefault, "controlPlane1"). + WithReplicas(3). WithVersion("v1.21.1"). Build(), want: []runtimehooksv1.Variable{ + { + Name: "location", + Value: toJSON("\"us-central\""), + }, + { + Name: "cpu", + Value: toJSON("8"), + }, + { + Name: runtimehooksv1.BuiltinsName, + Value: toJSONCompact(`{ + "controlPlane":{ + "version": "v1.21.1", + "name":"controlPlane1", + "replicas":3 + }}`), + }, + }, + }, + { + name: "Should calculate ControlPlane variables (without overrides)", + variableDefinitionsForPatch: map[string]bool{"location": true, "cpu": true}, + forPatch: "patch1", + controlPlaneTopology: &clusterv1.ControlPlaneTopology{ + Replicas: ptr.To[int32](3), + }, + controlPlane: builder.ControlPlane(metav1.NamespaceDefault, "controlPlane1"). + WithReplicas(3). + WithVersion("v1.21.1"). + Build(), + want: []runtimehooksv1.Variable{ + { + Name: runtimehooksv1.BuiltinsName, + Value: toJSONCompact(`{ + "controlPlane":{ + "version": "v1.21.1", + "name":"controlPlane1", + "replicas":3 + }}`), + }, + }, + }, + { + name: "Should calculate ControlPlane variables, replicas not set", + forPatch: "patch1", + variableDefinitionsForPatch: map[string]bool{"location": true, "cpu": true}, + controlPlaneTopology: &clusterv1.ControlPlaneTopology{ + Variables: &clusterv1.ControlPlaneVariables{ + Overrides: []clusterv1.ClusterVariable{ + { + Name: "location", + Value: toJSON("\"us-central\""), + }, + { + Name: "cpu", + Value: toJSON("8"), + }, + }, + }, + }, + controlPlane: builder.ControlPlane(metav1.NamespaceDefault, "controlPlane1"). + WithVersion("v1.21.1"). + Build(), + want: []runtimehooksv1.Variable{ + { + Name: "location", + Value: toJSON("\"us-central\""), + }, + { + Name: "cpu", + Value: toJSON("8"), + }, { Name: runtimehooksv1.BuiltinsName, Value: toJSONCompact(`{ @@ -448,9 +576,23 @@ func TestControlPlane(t *testing.T) { }, }, { - name: "Should calculate ControlPlane variables with InfrastructureMachineTemplate", + name: "Should calculate ControlPlane variables with InfrastructureMachineTemplate", + variableDefinitionsForPatch: map[string]bool{"location": true, "cpu": true}, + forPatch: "patch1", controlPlaneTopology: &clusterv1.ControlPlaneTopology{ Replicas: ptr.To[int32](3), + Variables: &clusterv1.ControlPlaneVariables{ + Overrides: []clusterv1.ClusterVariable{ + { + Name: "location", + Value: toJSON("\"us-central\""), + }, + { + Name: "cpu", + Value: toJSON("8"), + }, + }, + }, }, controlPlane: builder.ControlPlane(metav1.NamespaceDefault, "controlPlane1"). WithReplicas(3). @@ -459,6 +601,14 @@ func TestControlPlane(t *testing.T) { controlPlaneInfrastructureMachineTemplate: builder.InfrastructureMachineTemplate(metav1.NamespaceDefault, "controlPlaneInfrastructureMachineTemplate1"). Build(), want: []runtimehooksv1.Variable{ + { + Name: "location", + Value: toJSON("\"us-central\""), + }, + { + Name: "cpu", + Value: toJSON("8"), + }, { Name: runtimehooksv1.BuiltinsName, Value: toJSONCompact(`{ @@ -480,7 +630,7 @@ func TestControlPlane(t *testing.T) { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) - got, err := ControlPlane(tt.controlPlaneTopology, tt.controlPlane, tt.controlPlaneInfrastructureMachineTemplate) + got, err := ControlPlane(tt.controlPlaneTopology, tt.controlPlane, tt.controlPlaneInfrastructureMachineTemplate, tt.forPatch, tt.variableDefinitionsForPatch) g.Expect(err).ToNot(HaveOccurred()) g.Expect(got).To(BeComparableTo(tt.want)) }) diff --git a/internal/test/builder/builders.go b/internal/test/builder/builders.go index 1d7a2bb6da56..fbb932312a4b 100644 --- a/internal/test/builder/builders.go +++ b/internal/test/builder/builders.go @@ -114,12 +114,13 @@ func (c *ClusterBuilder) Build() *clusterv1.Cluster { // ClusterTopologyBuilder contains the fields needed to build a testable ClusterTopology. type ClusterTopologyBuilder struct { - class string - workers *clusterv1.WorkersTopology - version string - controlPlaneReplicas int32 - controlPlaneMHC *clusterv1.MachineHealthCheckTopology - variables []clusterv1.ClusterVariable + class string + workers *clusterv1.WorkersTopology + version string + controlPlaneReplicas int32 + controlPlaneMHC *clusterv1.MachineHealthCheckTopology + variables []clusterv1.ClusterVariable + controlPlaneVariables []clusterv1.ClusterVariable } // ClusterTopology returns a ClusterTopologyBuilder. @@ -147,6 +148,12 @@ func (c *ClusterTopologyBuilder) WithControlPlaneReplicas(replicas int32) *Clust return c } +// WithControlPlaneVariables adds the passed variable values to the ClusterTopologyBuilder. +func (c *ClusterTopologyBuilder) WithControlPlaneVariables(variables ...clusterv1.ClusterVariable) *ClusterTopologyBuilder { + c.controlPlaneVariables = variables + return c +} + // WithControlPlaneMachineHealthCheck adds MachineHealthCheckTopology used as the MachineHealthCheck value. func (c *ClusterTopologyBuilder) WithControlPlaneMachineHealthCheck(mhc *clusterv1.MachineHealthCheckTopology) *ClusterTopologyBuilder { c.controlPlaneMHC = mhc @@ -173,7 +180,7 @@ func (c *ClusterTopologyBuilder) WithVariables(vars ...clusterv1.ClusterVariable // Build returns a testable cluster Topology object with any values passed to the builder. func (c *ClusterTopologyBuilder) Build() *clusterv1.Topology { - return &clusterv1.Topology{ + t := &clusterv1.Topology{ Class: c.class, Workers: c.workers, Version: c.version, @@ -183,6 +190,14 @@ func (c *ClusterTopologyBuilder) Build() *clusterv1.Topology { }, Variables: c.variables, } + + if len(c.controlPlaneVariables) > 0 { + t.ControlPlane.Variables = &clusterv1.ControlPlaneVariables{ + Overrides: c.controlPlaneVariables, + } + } + + return t } // MachineDeploymentTopologyBuilder holds the values needed to create a testable MachineDeploymentTopology. diff --git a/internal/test/builder/zz_generated.deepcopy.go b/internal/test/builder/zz_generated.deepcopy.go index 73cee564d01a..77a9c44bf04d 100644 --- a/internal/test/builder/zz_generated.deepcopy.go +++ b/internal/test/builder/zz_generated.deepcopy.go @@ -224,6 +224,13 @@ func (in *ClusterTopologyBuilder) DeepCopyInto(out *ClusterTopologyBuilder) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.controlPlaneVariables != nil { + in, out := &in.controlPlaneVariables, &out.controlPlaneVariables + *out = make([]v1beta1.ClusterVariable, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterTopologyBuilder. diff --git a/internal/topology/variables/cluster_variable_validation.go b/internal/topology/variables/cluster_variable_validation.go index 1b4a07772640..e9cc24c97496 100644 --- a/internal/topology/variables/cluster_variable_validation.go +++ b/internal/topology/variables/cluster_variable_validation.go @@ -35,6 +35,11 @@ func ValidateClusterVariables(values []clusterv1.ClusterVariable, definitions [] return validateClusterVariables(values, definitions, true, fldPath) } +// ValidateControlPlaneVariables validates ControlPLane variables. +func ValidateControlPlaneVariables(values []clusterv1.ClusterVariable, definitions []clusterv1.ClusterClassStatusVariable, fldPath *field.Path) field.ErrorList { + return validateClusterVariables(values, definitions, false, fldPath) +} + // ValidateMachineVariables validates MachineDeployment and MachinePool variables. func ValidateMachineVariables(values []clusterv1.ClusterVariable, definitions []clusterv1.ClusterClassStatusVariable, fldPath *field.Path) field.ErrorList { return validateClusterVariables(values, definitions, false, fldPath) diff --git a/internal/webhooks/cluster.go b/internal/webhooks/cluster.go index beec7e1a0444..8fd06fa90837 100644 --- a/internal/webhooks/cluster.go +++ b/internal/webhooks/cluster.go @@ -719,9 +719,19 @@ func DefaultAndValidateVariables(cluster *clusterv1.Cluster, clusterClass *clust // Variables must be validated in the defaulting webhook. Variable definitions are stored in the ClusterClass status // and are patched in the ClusterClass reconcile. + + // Validate cluster-wide variables. allErrs = append(allErrs, variables.ValidateClusterVariables(cluster.Spec.Topology.Variables, clusterClass.Status.Variables, field.NewPath("spec", "topology", "variables"))...) + + // Validate ControlPlane variable overrides. + if cluster.Spec.Topology.ControlPlane.Variables != nil && len(cluster.Spec.Topology.ControlPlane.Variables.Overrides) > 0 { + allErrs = append(allErrs, variables.ValidateControlPlaneVariables(cluster.Spec.Topology.ControlPlane.Variables.Overrides, clusterClass.Status.Variables, + field.NewPath("spec", "topology", "controlPlane", "variables", "overrides"))...) + } + if cluster.Spec.Topology.Workers != nil { + // Validate MachineDeployment variable overrides. for i, md := range cluster.Spec.Topology.Workers.MachineDeployments { // Continue if there are no variable overrides. if md.Variables == nil || len(md.Variables.Overrides) == 0 { @@ -730,6 +740,8 @@ func DefaultAndValidateVariables(cluster *clusterv1.Cluster, clusterClass *clust allErrs = append(allErrs, variables.ValidateMachineVariables(md.Variables.Overrides, clusterClass.Status.Variables, field.NewPath("spec", "topology", "workers", "machineDeployments").Index(i).Child("variables", "overrides"))...) } + + // Validate MachinePool variable overrides. for i, mp := range cluster.Spec.Topology.Workers.MachinePools { // Continue if there are no variable overrides. if mp.Variables == nil || len(mp.Variables.Overrides) == 0 { @@ -751,6 +763,8 @@ func DefaultVariables(cluster *clusterv1.Cluster, clusterClass *clusterv1.Cluste if clusterClass == nil { return field.ErrorList{field.InternalError(field.NewPath(""), errors.New("ClusterClass can not be nil"))} } + + // Default cluster-wide variables. defaultedVariables, errs := variables.DefaultClusterVariables(cluster.Spec.Topology.Variables, clusterClass.Status.Variables, field.NewPath("spec", "topology", "variables")) if len(errs) > 0 { @@ -759,7 +773,19 @@ func DefaultVariables(cluster *clusterv1.Cluster, clusterClass *clusterv1.Cluste cluster.Spec.Topology.Variables = defaultedVariables } + // Default ControlPlane variable overrides. + if cluster.Spec.Topology.ControlPlane.Variables != nil && len(cluster.Spec.Topology.ControlPlane.Variables.Overrides) > 0 { + defaultedVariables, errs := variables.DefaultMachineVariables(cluster.Spec.Topology.ControlPlane.Variables.Overrides, clusterClass.Status.Variables, + field.NewPath("spec", "topology", "controlPlane", "variables", "overrides")) + if len(errs) > 0 { + allErrs = append(allErrs, errs...) + } else { + cluster.Spec.Topology.ControlPlane.Variables.Overrides = defaultedVariables + } + } + if cluster.Spec.Topology.Workers != nil { + // Default MachineDeployment variable overrides. for i, md := range cluster.Spec.Topology.Workers.MachineDeployments { // Continue if there are no variable overrides. if md.Variables == nil || len(md.Variables.Overrides) == 0 { @@ -773,6 +799,8 @@ func DefaultVariables(cluster *clusterv1.Cluster, clusterClass *clusterv1.Cluste md.Variables.Overrides = defaultedVariables } } + + // Default MachinePool variable overrides. for i, mp := range cluster.Spec.Topology.Workers.MachinePools { // Continue if there are no variable overrides. if mp.Variables == nil || len(mp.Variables.Overrides) == 0 { diff --git a/internal/webhooks/cluster_test.go b/internal/webhooks/cluster_test.go index 85e8b10e8947..393505cacce5 100644 --- a/internal/webhooks/cluster_test.go +++ b/internal/webhooks/cluster_test.go @@ -228,6 +228,7 @@ func TestClusterDefaultAndValidateVariables(t *testing.T) { }). Build(), topology: &clusterv1.Topology{ + ControlPlane: clusterv1.ControlPlaneTopology{}, Workers: &clusterv1.WorkersTopology{ MachineDeployments: []clusterv1.MachineDeploymentTopology{ { @@ -244,6 +245,9 @@ func TestClusterDefaultAndValidateVariables(t *testing.T) { }, }, expect: &clusterv1.Topology{ + ControlPlane: clusterv1.ControlPlaneTopology{ + // "location" has not been added to .variables.overrides. + }, Workers: &clusterv1.WorkersTopology{ MachineDeployments: []clusterv1.MachineDeploymentTopology{ { @@ -304,6 +308,16 @@ func TestClusterDefaultAndValidateVariables(t *testing.T) { }). Build(), topology: &clusterv1.Topology{ + ControlPlane: clusterv1.ControlPlaneTopology{ + Variables: &clusterv1.ControlPlaneVariables{ + Overrides: []clusterv1.ClusterVariable{ + { + Name: "httpProxy", + Value: apiextensionsv1.JSON{Raw: []byte(`{"enabled":true}`)}, + }, + }, + }, + }, Workers: &clusterv1.WorkersTopology{ MachineDeployments: []clusterv1.MachineDeploymentTopology{ { @@ -342,6 +356,17 @@ func TestClusterDefaultAndValidateVariables(t *testing.T) { }, }, expect: &clusterv1.Topology{ + ControlPlane: clusterv1.ControlPlaneTopology{ + Variables: &clusterv1.ControlPlaneVariables{ + Overrides: []clusterv1.ClusterVariable{ + { + Name: "httpProxy", + // url has been added by defaulting. + Value: apiextensionsv1.JSON{Raw: []byte(`{"enabled":true,"url":"http://localhost:3128"}`)}, + }, + }, + }, + }, Workers: &clusterv1.WorkersTopology{ MachineDeployments: []clusterv1.MachineDeploymentTopology{ { @@ -631,7 +656,43 @@ func TestClusterDefaultAndValidateVariables(t *testing.T) { wantErr: true, }, { - name: "should fail when variable override is invalid", + name: "should fail when ControlPlane variable override is invalid", + clusterClass: builder.ClusterClass(metav1.NamespaceDefault, "class1"). + WithStatusVariables(clusterv1.ClusterClassStatusVariable{ + Name: "cpu", + Definitions: []clusterv1.ClusterClassStatusVariableDefinition{ + { + Required: true, + From: clusterv1.VariableDefinitionFromInline, + Schema: clusterv1.VariableSchema{ + OpenAPIV3Schema: clusterv1.JSONSchemaProps{ + Type: "integer", + }, + }, + }, + }}).Build(), + topology: builder.ClusterTopology(). + WithVariables(clusterv1.ClusterVariable{ + Name: "cpu", + Value: apiextensionsv1.JSON{Raw: []byte(`2`)}, + }). + WithControlPlaneVariables(clusterv1.ClusterVariable{ + Name: "cpu", + // This value is invalid. + Value: apiextensionsv1.JSON{Raw: []byte(`"text"`)}, + }). + WithMachineDeployment(builder.MachineDeploymentTopology("workers1"). + WithClass("aa"). + Build()). + WithMachinePool(builder.MachinePoolTopology("workers1"). + WithClass("aa"). + Build()). + Build(), + expect: builder.ClusterTopology().Build(), + wantErr: true, + }, + { + name: "should fail when MD variable override is invalid", clusterClass: builder.ClusterClass(metav1.NamespaceDefault, "class1"). WithStatusVariables(clusterv1.ClusterClassStatusVariable{ Name: "cpu", @@ -654,14 +715,47 @@ func TestClusterDefaultAndValidateVariables(t *testing.T) { WithMachineDeployment(builder.MachineDeploymentTopology("workers1"). WithClass("aa"). WithVariables(clusterv1.ClusterVariable{ - Name: "cpu", + Name: "cpu", + // This value is invalid. Value: apiextensionsv1.JSON{Raw: []byte(`"text"`)}, }). Build()). + WithMachinePool(builder.MachinePoolTopology("workers1"). + WithClass("aa"). + Build()). + Build(), + expect: builder.ClusterTopology().Build(), + wantErr: true, + }, + { + name: "should fail when MP variable override is invalid", + clusterClass: builder.ClusterClass(metav1.NamespaceDefault, "class1"). + WithStatusVariables(clusterv1.ClusterClassStatusVariable{ + Name: "cpu", + Definitions: []clusterv1.ClusterClassStatusVariableDefinition{ + { + Required: true, + From: clusterv1.VariableDefinitionFromInline, + Schema: clusterv1.VariableSchema{ + OpenAPIV3Schema: clusterv1.JSONSchemaProps{ + Type: "integer", + }, + }, + }, + }}).Build(), + topology: builder.ClusterTopology(). + WithVariables(clusterv1.ClusterVariable{ + Name: "cpu", + Value: apiextensionsv1.JSON{Raw: []byte(`2`)}, + }). + WithMachineDeployment(builder.MachineDeploymentTopology("workers1"). + WithClass("aa"). + Build()). WithMachinePool(builder.MachinePoolTopology("workers1"). WithClass("aa"). WithVariables(clusterv1.ClusterVariable{ - Name: "cpu", + Name: "cpu", + // This value is invalid. Value: apiextensionsv1.JSON{Raw: []byte(`"text"`)}, }). Build()). @@ -692,7 +786,7 @@ func TestClusterDefaultAndValidateVariables(t *testing.T) { Name: "cpu", Value: apiextensionsv1.JSON{Raw: []byte(`2`)}, }). - // Variable is not required in MachineDeployment or MachinePool topologies. + // Variable is not required in ControlPlane, MachineDeployment or MachinePool topologies. Build(), expect: builder.ClusterTopology(). WithClass("foo"). @@ -701,7 +795,7 @@ func TestClusterDefaultAndValidateVariables(t *testing.T) { Name: "cpu", Value: apiextensionsv1.JSON{Raw: []byte(`2`)}, }). - // Variable is not required in MachineDeployment or MachinePool topologies. + // Variable is not required in ControlPlane, MachineDeployment or MachinePool topologies. Build(), }, { @@ -729,6 +823,10 @@ func TestClusterDefaultAndValidateVariables(t *testing.T) { Name: "cpu", Value: apiextensionsv1.JSON{Raw: []byte(`2`)}, }). + WithControlPlaneVariables(clusterv1.ClusterVariable{ + Name: "cpu", + Value: apiextensionsv1.JSON{Raw: []byte(`2`)}, + }). WithMachineDeployment(builder.MachineDeploymentTopology("workers1"). WithClass("md1"). WithVariables(clusterv1.ClusterVariable{ @@ -751,6 +849,10 @@ func TestClusterDefaultAndValidateVariables(t *testing.T) { Name: "cpu", Value: apiextensionsv1.JSON{Raw: []byte(`2`)}, }). + WithControlPlaneVariables(clusterv1.ClusterVariable{ + Name: "cpu", + Value: apiextensionsv1.JSON{Raw: []byte(`2`)}, + }). WithMachineDeployment(builder.MachineDeploymentTopology("workers1"). WithClass("md1"). WithVariables(clusterv1.ClusterVariable{ @@ -776,6 +878,8 @@ func TestClusterDefaultAndValidateVariables(t *testing.T) { Name: "cpu", Definitions: []clusterv1.ClusterClassStatusVariableDefinition{ { + // For optional variables, it is optional to set top-level variables + // but overrides can be set even if the top-level variables are not set. Required: false, From: clusterv1.VariableDefinitionFromInline, Schema: clusterv1.VariableSchema{ @@ -788,6 +892,10 @@ func TestClusterDefaultAndValidateVariables(t *testing.T) { topology: builder.ClusterTopology(). WithClass("foo"). WithVersion("v1.19.1"). + WithControlPlaneVariables(clusterv1.ClusterVariable{ + Name: "cpu", + Value: apiextensionsv1.JSON{Raw: []byte(`2`)}, + }). WithMachineDeployment(builder.MachineDeploymentTopology("workers1"). WithClass("md1"). WithVariables(clusterv1.ClusterVariable{ @@ -807,6 +915,10 @@ func TestClusterDefaultAndValidateVariables(t *testing.T) { WithClass("foo"). WithVersion("v1.19.1"). WithVariables([]clusterv1.ClusterVariable{}...). + WithControlPlaneVariables(clusterv1.ClusterVariable{ + Name: "cpu", + Value: apiextensionsv1.JSON{Raw: []byte(`2`)}, + }). WithMachineDeployment(builder.MachineDeploymentTopology("workers1"). WithClass("md1"). WithVariables(clusterv1.ClusterVariable{