From 1438f669e6ad660a3278dd393c72ade668ca1e40 Mon Sep 17 00:00:00 2001 From: Cezar Sa Espinola Date: Thu, 27 Jun 2024 10:33:13 -0300 Subject: [PATCH 1/2] Fix XValidations flattening --- pkg/crd/flatten.go | 2 ++ pkg/crd/flatten_all_of_test.go | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/pkg/crd/flatten.go b/pkg/crd/flatten.go index 9224c26b2..9debd1b56 100644 --- a/pkg/crd/flatten.go +++ b/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(dstField, srcField)) // 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: diff --git a/pkg/crd/flatten_all_of_test.go b/pkg/crd/flatten_all_of_test.go index ae0499d45..dc87a7bb4 100644 --- a/pkg/crd/flatten_all_of_test.go +++ b/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: "rule1"}, {Rule: "rule2"}}}, + {XValidations: apiext.ValidationRules{{Rule: "rule3"}}}, + }, + } + 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() { From 43611064a1be33cfab6d25aa1b03bd11a1b39d3c Mon Sep 17 00:00:00 2001 From: Cezar Sa Espinola Date: Thu, 11 Jul 2024 11:53:35 -0300 Subject: [PATCH 2/2] Add integration test for xvalidation on field and struct --- pkg/crd/testdata/cronjob_types.go | 8 ++++++++ pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml | 9 +++++++++ 2 files changed, 17 insertions(+) diff --git a/pkg/crd/testdata/cronjob_types.go b/pkg/crd/testdata/cronjob_types.go index 50cd689b6..42af267ab 100644 --- a/pkg/crd/testdata/cronjob_types.go +++ b/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 diff --git a/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml b/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml index 95a5d870b..73157a560 100644 --- a/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml +++ b/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 even length + rule: self.size() % 2 == 0 + - message: must have good prefix + rule: self.startsWith('good-') structWithSeveralFields: description: A struct that can only be entirely replaced properties: