fixup! feat: Support IntOrString variables

kubernetes-sigs · Sep 26, 2024 · 47f98f0 · 47f98f0
1 parent ffe3680
commit 47f98f0
Show file tree

Hide file tree

Showing 7 changed files with 96 additions and 153 deletions.
diff --git a/api/v1beta1/clusterclass_types.go b/api/v1beta1/clusterclass_types.go
@@ -584,6 +584,21 @@ type JSONSchemaProps struct {
 	// +optional
 	XMetadata *VariableSchemaMetadata `json:"x-metadata,omitempty"`
 
+	// x-kubernetes-int-or-string specifies that this value is
+	// either an integer or a string. If this is true, an empty
+	// type is allowed and type as child of anyOf is permitted
+	// if following one of the following patterns:
+	//
+	// 1) anyOf:
+	//    - type: integer
+	//    - type: string
+	// 2) allOf:
+	//    - anyOf:
+	//      - type: integer
+	//      - type: string
+	//    - ... zero or more
+	XIntOrString bool `json:"x-kubernetes-int-or-string,omitempty"`
+
 	// AllOf specifies that the variable must validate against all of the subschemas in the array.
 	// NOTE: This field uses PreserveUnknownFields and Schemaless,
 	// because recursive validation is not possible.

diff --git a/api/v1beta1/zz_generated.openapi.go b/api/v1beta1/zz_generated.openapi.go
diff --git a/config/crd/bases/cluster.x-k8s.io_clusterclasses.yaml b/config/crd/bases/cluster.x-k8s.io_clusterclasses.yaml
diff --git a/internal/topology/variables/cluster_variable_validation_test.go b/internal/topology/variables/cluster_variable_validation_test.go
@@ -2190,155 +2190,6 @@ func Test_ValidateClusterVariable(t *testing.T) {
 					Raw: []byte(`{"propertyA":true, "propertyB":true}`),
 				},
 			},
-		}, {
-			name: "pass & fail correctly for int-or-string in oneOf/anyOf/allOf schemas with int values",
-			clusterClassVariable: &clusterv1.ClusterClassVariable{
-				Name: "test",
-				Schema: clusterv1.VariableSchema{
-					OpenAPIV3Schema: clusterv1.JSONSchemaProps{
-						Type: "object",
-						Properties: map[string]clusterv1.JSONSchemaProps{
-							"anyOfExampleField": { // Valid variant
-								AnyOf: []clusterv1.JSONSchemaProps{{
-									Type: "integer",
-								}, {
-									Type: "string",
-								}},
-							},
-							"allOfExampleField": { // Invalid variant
-								AllOf: []clusterv1.JSONSchemaProps{{
-									Type: "integer",
-								}, {
-									Type: "string",
-								}},
-							},
-							"oneOfExampleField": { // Valid variant
-								OneOf: []clusterv1.JSONSchemaProps{{
-									Type: "integer",
-								}, {
-									Type: "string",
-								}},
-							},
-						},
-					},
-				},
-			},
-			clusterVariable: &clusterv1.ClusterVariable{
-				Name: "test",
-				Value: apiextensionsv1.JSON{
-					Raw: []byte(`{"anyOfExampleField":42, "allOfExampleField":42, "oneOfExampleField":42}`),
-				},
-			},
-			wantErrs: []validationMatch{
-				invalidType(`allOfExampleField in body must be of type string: "integer"`, "spec.topology.variables[test].value.allOfExampleField"),
-				invalid(
-					`Invalid value: "{\"anyOfExampleField\":42, \"allOfExampleField\":42, \"oneOfExampleField\":42}": "allOfExampleField" must validate all the schemas (allOf)`,
-					"spec.topology.variables[test].value",
-				),
-			},
-		}, {
-			name: "pass & fail correctly for int-or-string in oneOf/anyOf/allOf schemas with string values",
-			clusterClassVariable: &clusterv1.ClusterClassVariable{
-				Name: "test",
-				Schema: clusterv1.VariableSchema{
-					OpenAPIV3Schema: clusterv1.JSONSchemaProps{
-						Type: "object",
-						Properties: map[string]clusterv1.JSONSchemaProps{
-							"anyOfExampleField": { // Valid variant
-								AnyOf: []clusterv1.JSONSchemaProps{{
-									Type: "integer",
-								}, {
-									Type: "string",
-								}},
-							},
-							"allOfExampleField": { // Invalid variant
-								AllOf: []clusterv1.JSONSchemaProps{{
-									Type: "integer",
-								}, {
-									Type: "string",
-								}},
-							},
-							"oneOfExampleField": { // Valid variant
-								OneOf: []clusterv1.JSONSchemaProps{{
-									Type: "integer",
-								}, {
-									Type: "string",
-								}},
-							},
-						},
-					},
-				},
-			},
-			clusterVariable: &clusterv1.ClusterVariable{
-				Name: "test",
-				Value: apiextensionsv1.JSON{
-					Raw: []byte(`{"anyOfExampleField":"42", "allOfExampleField":"42", "oneOfExampleField":"42"}`),
-				},
-			},
-			wantErrs: []validationMatch{
-				invalidType(`allOfExampleField in body must be of type integer: "string"`, "spec.topology.variables[test].value.allOfExampleField"),
-				invalid(
-					`Invalid value: "{\"anyOfExampleField\":\"42\", \"allOfExampleField\":\"42\", \"oneOfExampleField\":\"42\"}": "allOfExampleField" must validate all the schemas (allOf)`,
-					"spec.topology.variables[test].value",
-				),
-			},
-		}, {
-			name: "fail for int-or-string in oneOf/anyOf/allOf schemas with object values",
-			clusterClassVariable: &clusterv1.ClusterClassVariable{
-				Name: "test",
-				Schema: clusterv1.VariableSchema{
-					OpenAPIV3Schema: clusterv1.JSONSchemaProps{
-						Type: "object",
-						Properties: map[string]clusterv1.JSONSchemaProps{
-							"anyOfExampleField": { // Valid variant
-								AnyOf: []clusterv1.JSONSchemaProps{{
-									Type: "integer",
-								}, {
-									Type: "string",
-								}},
-							},
-							"allOfExampleField": { // Invalid variant
-								AllOf: []clusterv1.JSONSchemaProps{{
-									Type: "integer",
-								}, {
-									Type: "string",
-								}},
-							},
-							"oneOfExampleField": { // Valid variant
-								OneOf: []clusterv1.JSONSchemaProps{{
-									Type: "integer",
-								}, {
-									Type: "string",
-								}},
-							},
-						},
-					},
-				},
-			},
-			clusterVariable: &clusterv1.ClusterVariable{
-				Name: "test",
-				Value: apiextensionsv1.JSON{
-					Raw: []byte(`{"anyOfExampleField":{}, "allOfExampleField":{}, "oneOfExampleField":{}}`),
-				},
-			},
-			wantErrs: []validationMatch{
-				invalid(
-					`Invalid value: "{\"anyOfExampleField\":{}, \"allOfExampleField\":{}, \"oneOfExampleField\":{}}": "anyOfExampleField" must validate at least one schema (anyOf)`,
-					"spec.topology.variables[test].value",
-				),
-				invalidType(`anyOfExampleField in body must be of type integer: "object"`, "spec.topology.variables[test].value.anyOfExampleField"),
-				invalidType(`allOfExampleField in body must be of type integer: "object"`, "spec.topology.variables[test].value.allOfExampleField"),
-				invalidType(`allOfExampleField in body must be of type string: "object"`, "spec.topology.variables[test].value.allOfExampleField"),
-				invalid(
-					`Invalid value: "{\"anyOfExampleField\":{}, \"allOfExampleField\":{}, \"oneOfExampleField\":{}}": "allOfExampleField" must validate all the schemas (allOf). None validated`,
-					"spec.topology.variables[test].value",
-				),
-				invalid(
-					`Invalid value: "{\"anyOfExampleField\":{}, \"allOfExampleField\":{}, \"oneOfExampleField\":{}}": "oneOfExampleField" must validate one and only one schema (oneOf). Found none valid`,
-					"spec.topology.variables[test].value",
-				),
-				invalidType(`oneOfExampleField in body must be of type integer: "object"`, "spec.topology.variables[test].value.oneOfExampleField"),
-			},
 		}, {
 			name: "Valid object for int-or-string (resource.Quantity)",
 			clusterClassVariable: &clusterv1.ClusterClassVariable{
@@ -2348,6 +2199,7 @@ func Test_ValidateClusterVariable(t *testing.T) {
 					OpenAPIV3Schema: clusterv1.JSONSchemaProps{
 						Type: "array",
 						Items: &clusterv1.JSONSchemaProps{
+							XIntOrString: true,
 							AnyOf: []clusterv1.JSONSchemaProps{{
 								Type: "integer",
 							}, {

diff --git a/internal/topology/variables/clusterclass_variable_validation_test.go b/internal/topology/variables/clusterclass_variable_validation_test.go
@@ -2746,24 +2746,32 @@ func Test_ValidateClusterClassVariable(t *testing.T) {
 					OpenAPIV3Schema: clusterv1.JSONSchemaProps{
 						Type: "object",
 						Properties: map[string]clusterv1.JSONSchemaProps{
-							"anyOfExampleField": { // Only valid variant
-								Type: "object",
+							"anyOfExampleField": { // Valid variant
+								XIntOrString: true,
 								AnyOf: []clusterv1.JSONSchemaProps{{
 									Type: "integer",
 								}, {
 									Type: "string",
 								}},
 							},
+							"allOfExampleFieldWithAnyOf": { // Valid variant
+								XIntOrString: true,
+								AllOf: []clusterv1.JSONSchemaProps{{
+									AnyOf: []clusterv1.JSONSchemaProps{{
+										Type: "integer",
+									}, {
+										Type: "string",
+									}},
+								}},
+							},
 							"allOfExampleField": {
-								Type: "object",
 								AllOf: []clusterv1.JSONSchemaProps{{
 									Type: "integer",
 								}, {
 									Type: "string",
 								}},
 							},
 							"oneOfExampleField": {
-								Type: "object",
 								OneOf: []clusterv1.JSONSchemaProps{{
 									Type: "integer",
 								}, {
@@ -2777,8 +2785,10 @@ func Test_ValidateClusterClassVariable(t *testing.T) {
 			wantErrs: []validationMatch{
 				forbidden("must be empty to be structural", "spec.variables[variableA].schema.openAPIV3Schema.properties[allOfExampleField].allOf[0].type"),
 				forbidden("must be empty to be structural", "spec.variables[variableA].schema.openAPIV3Schema.properties[allOfExampleField].allOf[1].type"),
+				required("must not be empty for specified object fields", "spec.variables[variableA].schema.openAPIV3Schema.properties[allOfExampleField].type"),
 				forbidden("must be empty to be structural", "spec.variables[variableA].schema.openAPIV3Schema.properties[oneOfExampleField].oneOf[0].type"),
 				forbidden("must be empty to be structural", "spec.variables[variableA].schema.openAPIV3Schema.properties[oneOfExampleField].oneOf[1].type"),
+				required("must not be empty for specified object fields", "spec.variables[variableA].schema.openAPIV3Schema.properties[oneOfExampleField].type"),
 			},
 		},
 	}

diff --git a/internal/topology/variables/schema.go b/internal/topology/variables/schema.go
@@ -49,6 +49,7 @@ func convertToAPIExtensionsJSONSchemaProps(schema *clusterv1.JSONSchemaProps, fl
 		Pattern:          schema.Pattern,
 		ExclusiveMaximum: schema.ExclusiveMaximum,
 		ExclusiveMinimum: schema.ExclusiveMinimum,
+		XIntOrString:     schema.XIntOrString,
 	}
 
 	// Only set XPreserveUnknownFields to true if it's true.

diff --git a/internal/topology/variables/schema_test.go b/internal/topology/variables/schema_test.go
@@ -376,6 +376,30 @@ func Test_convertToAPIExtensionsJSONSchemaProps(t *testing.T) {
 					},
 				},
 			},
+		}, {
+			name: "pass for schema validation with XIntOrString",
+			schema: &clusterv1.JSONSchemaProps{
+				Items: &clusterv1.JSONSchemaProps{
+					XIntOrString: true,
+					AnyOf: []clusterv1.JSONSchemaProps{{
+						Type: "integer",
+					}, {
+						Type: "string",
+					}},
+				},
+			},
+			want: &apiextensions.JSONSchemaProps{
+				Items: &apiextensions.JSONSchemaPropsOrArray{
+					Schema: &apiextensions.JSONSchemaProps{
+						XIntOrString: true,
+						AnyOf: []apiextensions.JSONSchemaProps{{
+							Type: "integer",
+						}, {
+							Type: "string",
+						}},
+					},
+				},
+			},
 		},
 	}
 	for _, tt := range tests {