🐛 Fix XValidations flattening (#998)

* Fix XValidations flattening

* Add integration test for xvalidation on field and struct

* Reverse order of XValidations when flattening

Author: cezarsa

pkg/crd/flatten.go

@@ -147,6 +147,8 @@ func flattenAllOfInto(dst *apiext.JSONSchemaProps, src apiext.JSONSchemaProps, e
 			dstField.Set(srcField)
 		case "XMapType":
 			dstField.Set(srcField)
+		case "XValidations":
+			dstField.Set(reflect.AppendSlice(srcField, dstField))
 		// NB(directxman12): no need to explicitly handle nullable -- false is considered to be the zero value
 		// TODO(directxman12): src isn't necessarily the field value -- it's just the most recent allOf entry
 		default:

pkg/crd/flatten_all_of_test.go

@@ -241,6 +241,25 @@ var _ = Describe("AllOf Flattening", func() {
 				},
 			}))
 		})
+
+		It("should merge XValidation fields", func() {
+			By("flattening a schema with multiple validation fields")
+			original := &apiext.JSONSchemaProps{
+				AllOf: []apiext.JSONSchemaProps{
+					{XValidations: apiext.ValidationRules{{Rule: "rule2"}, {Rule: "rule3"}}},
+					{XValidations: apiext.ValidationRules{{Rule: "rule1"}}},
+				},
+			}
+			flattened := crd.FlattenEmbedded(original, errRec)
+			Expect(errRec.FirstError()).NotTo(HaveOccurred())
+
+			By("ensuring that the result lists all validation rules")
+			Expect(flattened).To(Equal(&apiext.JSONSchemaProps{
+				XValidations: []apiext.ValidationRule{
+					{Rule: "rule1"}, {Rule: "rule2"}, {Rule: "rule3"},
+				},
+			}))
+		})
 	})
 
 	It("should skip Title, Description, Example, and ExternalDocs, assuming they've been merged pre-AllOf flattening", func() {

pkg/crd/testdata/cronjob_types.go

@@ -303,6 +303,10 @@ type CronJobSpec struct {
 	// +kubebuilder:validation:XValidation:rule="self.size() % 2 == 0",messageExpression="'Length has to be even but is ' + len(self.stringWithEvenLengthAndMessageExpression) + ' instead'"
 	StringWithEvenLengthAndMessageExpression string `json:"stringWithEvenLengthAndMessageExpression,omitempty"`
 
+	// Test of the expression-based validation on both field and type.
+	// +kubebuilder:validation:XValidation:rule="self.startsWith('good-')",message="must have good prefix"
+	StringWithEvenLengthAndGoodPrefix StringEvenType `json:"stringWithEvenLengthAndGoodPrefix,omitempty"`
+
 	// Test that we can add a forbidden field using XValidation Reason and FieldPath.
 	// The validation is applied to the spec struct itself and not the field.
 	ForbiddenInt int `json:"forbiddenInt,omitempty"`
@@ -585,6 +589,10 @@ const (
 	ReplaceConcurrent ConcurrencyPolicy = "Replace"
 )
 
+// StringEvenType is a type that includes an expression-based validation.
+// +kubebuilder:validation:XValidation:rule="self.size() % 2 == 0",message="must have even length"
+type StringEvenType string
+
 // CronJobStatus defines the observed state of CronJob
 type CronJobStatus struct {
 	// INSERT ADDITIONAL STATUS FIELD - define observed state of cluster

pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml

@@ -6840,6 +6840,15 @@ spec:
                 - messageExpression: '''Length has to be even but is '' + len(self.stringWithEvenLengthAndMessageExpression)
                     + '' instead'''
                   rule: self.size() % 2 == 0
+              stringWithEvenLengthAndGoodPrefix:
+                description: Test of the expression-based validation on both field
+                  and type.
+                type: string
+                x-kubernetes-validations:
+                - message: must have good prefix
+                  rule: self.startsWith('good-')
+                - message: must have even length
+                  rule: self.size() % 2 == 0
               structWithSeveralFields:
                 description: A struct that can only be entirely replaced
                 properties: