diff --git a/api/v1beta1/cluster_types.go b/api/v1beta1/cluster_types.go index b71033fbf7d7..74bf062129f4 100644 --- a/api/v1beta1/cluster_types.go +++ b/api/v1beta1/cluster_types.go @@ -162,8 +162,10 @@ type ClusterVariable struct { // 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's not possible to have no type field. + // 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 + // Note: This field has to be nullable, so allow setting nullable variables to "null". + // +nullable Value apiextensionsv1.JSON `json:"value"` } diff --git a/config/crd/bases/cluster.x-k8s.io_clusters.yaml b/config/crd/bases/cluster.x-k8s.io_clusters.yaml index 6230bace8c0b..309a1bed8cb3 100644 --- a/config/crd/bases/cluster.x-k8s.io_clusters.yaml +++ b/config/crd/bases/cluster.x-k8s.io_clusters.yaml @@ -892,7 +892,10 @@ spec: 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''s not possible to have no type field. Ref: https://github.com/kubernetes-sigs/controller-tools/blob/d0e03a142d0ecdd5491593e941ee1d6b5d91dba6/pkg/crd/known_types.go#L106-L111' + 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 + Note: This field has to be nullable, so allow setting + nullable variables to "null".' + nullable: true x-kubernetes-preserve-unknown-fields: true required: - name diff --git a/internal/topology/variables/cluster_variable_defaulting_test.go b/internal/topology/variables/cluster_variable_defaulting_test.go index a3388a50f077..83400cba98ae 100644 --- a/internal/topology/variables/cluster_variable_defaulting_test.go +++ b/internal/topology/variables/cluster_variable_defaulting_test.go @@ -188,6 +188,38 @@ func Test_DefaultClusterVariables(t *testing.T) { }, }, }, + { + name: "Don't default variables set to null", + clusterClassVariables: []clusterv1.ClusterClassVariable{ + { + Name: "cpu", + Required: true, + Schema: clusterv1.VariableSchema{ + OpenAPIV3Schema: clusterv1.JSONSchemaProps{ + Type: "integer", + Default: &apiextensionsv1.JSON{Raw: []byte(`1`)}, + Nullable: true, + }, + }, + }, + }, + clusterVariables: []clusterv1.ClusterVariable{ + { + Name: "cpu", + Value: apiextensionsv1.JSON{ + Raw: nil, // A JSON with a nil value is the result of setting the variable value to "null" via YAML. + }, + }, + }, + want: []clusterv1.ClusterVariable{ + { + Name: "cpu", + Value: apiextensionsv1.JSON{ + Raw: nil, // A JSON with a nil value is the result of setting the variable value to "null" via YAML. + }, + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/internal/topology/variables/cluster_variable_validation.go b/internal/topology/variables/cluster_variable_validation.go index 034b8c600533..a175421b342c 100644 --- a/internal/topology/variables/cluster_variable_validation.go +++ b/internal/topology/variables/cluster_variable_validation.go @@ -91,9 +91,13 @@ func validateClusterVariablesDefined(clusterVariables map[string]*clusterv1.Clus func validateClusterVariable(clusterVariable *clusterv1.ClusterVariable, clusterClassVariable *clusterv1.ClusterClassVariable, fldPath *field.Path) field.ErrorList { // Parse JSON value. var variableValue interface{} - if err := json.Unmarshal(clusterVariable.Value.Raw, &variableValue); err != nil { - return field.ErrorList{field.Invalid(fldPath, string(clusterVariable.Value.Raw), - fmt.Sprintf("variable %q could not be parsed: %v", clusterVariable.Name, err))} + // Only try to unmarshal the clusterVariable if it is not nil, otherwise the variableValue is nil. + // Note: A clusterVariable with a nil value is the result of setting the variable value to "null" via YAML. + if clusterVariable.Value.Raw != nil { + if err := json.Unmarshal(clusterVariable.Value.Raw, &variableValue); err != nil { + return field.ErrorList{field.Invalid(fldPath, string(clusterVariable.Value.Raw), + fmt.Sprintf("variable %q could not be parsed: %v", clusterVariable.Name, err))} + } } // Convert schema to Kubernetes APIExtensions Schema. diff --git a/internal/topology/variables/cluster_variable_validation_test.go b/internal/topology/variables/cluster_variable_validation_test.go index 5228eb59991e..7751179ed076 100644 --- a/internal/topology/variables/cluster_variable_validation_test.go +++ b/internal/topology/variables/cluster_variable_validation_test.go @@ -340,6 +340,45 @@ func Test_ValidateClusterVariable(t *testing.T) { }, wantErr: true, }, + { + name: "Error if string is null", + clusterClassVariable: &clusterv1.ClusterClassVariable{ + Name: "location", + Required: true, + Schema: clusterv1.VariableSchema{ + OpenAPIV3Schema: clusterv1.JSONSchemaProps{ + Type: "string", + Nullable: false, + }, + }, + }, + clusterVariable: &clusterv1.ClusterVariable{ + Name: "location", + Value: apiextensionsv1.JSON{ + Raw: nil, // A JSON with a nil value is the result of setting the variable value to "null" via YAML. + }, + }, + wantErr: true, + }, + { + name: "Valid nullable string", + clusterClassVariable: &clusterv1.ClusterClassVariable{ + Name: "location", + Required: true, + Schema: clusterv1.VariableSchema{ + OpenAPIV3Schema: clusterv1.JSONSchemaProps{ + Type: "string", + Nullable: true, + }, + }, + }, + clusterVariable: &clusterv1.ClusterVariable{ + Name: "location", + Value: apiextensionsv1.JSON{ + Raw: nil, // A JSON with a nil value is the result of setting the variable value to "null" via YAML. + }, + }, + }, { name: "Valid enum",