✨ markers: add support for optionalOldSelf in XValidation marker #1150
Conversation
k8s-ci-robot added the cncf-cla: yes k8s-ci-robot added the size/XS 
everettraven force-pushed the feature/optional-oldself-xvalidation branch from 50be6ae to 1e35f95 Compare 
| Given previous PR to work on the path and reason that I did, this looks pretty much the same |
k8s-ci-robot added the lgtm 
| LGTM label has been added. Git tree hash: ab13142695d853aa9b83d90622888af8a971579a |
pkg/crd/testdata/cronjob_types.go Outdated
| | ||
| // CronJobSpec defines the desired state of CronJob | ||
| // +kubebuilder:validation:XValidation:rule="has(oldSelf.forbiddenInt) || !has(self.forbiddenInt)",message="forbiddenInt is not allowed",fieldPath=".forbiddenInt",reason="FieldValueForbidden" | ||
| // +kubebuilder:validation:XValidation:rule="has(oldSelf.forbiddenInt) || !has(self.forbiddenInt)",message="forbiddenInt is not allowed",fieldPath=".forbiddenInt",reason="FieldValueForbidden",optionalOldSelf=true |
There was a problem hiding this comment.
Does the rule need to change to account for the fact that oldSelf is now optional?
EDIT: I know it doesn't in order to actually test that this field propagates to the CRD, but it these examples are useful for illustrative purposes as well.
There was a problem hiding this comment.
Updated in the latest push to include a new field with an explicit example of oldSelf being optional.
everettraven force-pushed the feature/optional-oldself-xvalidation branch from 1e35f95 to d28c772 Compare k8s-ci-robot added size/S 
| /lgtm |
k8s-ci-robot added the lgtm | LGTM label has been added. Git tree hash: 8240734ad15205a1bd7965d262eac4af5d1cdfe0 |
| @sbueringer @alvaroaleman Either you able to help us get this merged please? |
pkg/crd/testdata/cronjob_types.go Outdated
| InlineAlias `json:",inline"` | ||
| | ||
| // Test that we can add a field that can only be set on updates using XValidation OptionalOldSelf. | ||
| // The validation is applied to the spec struct itself and not the field. |
There was a problem hiding this comment.
// The validation is applied to the spec struct itself and not the field.
I'm a bit confused about this statement. Is it correct?
There was a problem hiding this comment.
Whoops - forgot to update that after I decided to move it from the spec. Will update that soon
There was a problem hiding this comment.
Fixed in the latest push.
k8s-ci-robot added the do-not-merge/hold 
| /hold |
| /approve |
sbueringer added the approved everettraven force-pushed the feature/optional-oldself-xvalidation branch from d28c772 to 31acd94 Compare k8s-ci-robot removed the lgtm 
Signed-off-by: Bryce Palmer <bpalmer@redhat.com>
everettraven force-pushed the feature/optional-oldself-xvalidation branch from 31acd94 to 5e25270 Compare | [APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: everettraven, sbueringer The full list of commands accepted by this bot can be found here. The pull request process is described here Needs approval from an approver in each of these files: Approvers can indicate their approval by writing |
| /lgtm |
k8s-ci-robot added the lgtm | LGTM label has been added. Git tree hash: 100575e3126e862f349604addfcaeb8b2caf933d |
| /hold cancel |
k8s-ci-robot removed the do-not-merge/hold 
5 participants
Adds a new field to the
+kubebuilder:validation:XValidationmarker to specifyoptionalOldSelf. Adding this configuration option allows writing validation rules that behave one way on create vs update. For example, allow setting a field if it is an update, but don't allow the field to be set at creation.For more explicit information on the
optionalOldSelffield for validation rules, see https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#field-optional-oldself