Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

validate: Expose hooks to inject custom behavior during traversal #406

Merged
merged 6 commits into from
Jul 14, 2023

Conversation

alexzielenski
Copy link
Contributor

@alexzielenski alexzielenski commented Jun 12, 2023

Adds a new Option to the schema validator for users to provider custom constructors for subfield and subindex-validators created during traversal.

/cc @stts @apelisse @jpbetz

@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 12, 2023
@k8s-ci-robot k8s-ci-robot requested review from apelisse and jpbetz June 12, 2023 18:18
@k8s-ci-robot
Copy link
Contributor

@alexzielenski: GitHub didn't allow me to request PR reviews from the following users: stts.

Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

Proof of concept that adds a ratcheting validator to kube-openapi. Taking care to change code as minimally as possible.

More complicated/expansive tests forthcoming.

Ignores allOf, anyOf, not, oneOf for the purposes of ratcheting.

Still need to evaluate proper handling of required/dependentProperties and other cases which may not be caught by current DeepEqual.

/cc @stts @apelisse @jpbetz

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 12, 2023
@alexzielenski
Copy link
Contributor Author

/cc @sttts

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 29, 2023
makes usage consistent with other places, also makes moving to a new index for validation explicit
@apelisse
Copy link
Member

Looks good to me, Stefan, can you tag it?
/approve

@alexzielenski alexzielenski changed the title [WIP] add ratcheting validator validate: Expose hooks to inject custom behavior during traversal Jul 11, 2023
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 11, 2023
@jpbetz
Copy link
Contributor

jpbetz commented Jul 12, 2023

/approve

I'll defer to @sttts for lgtm

pkg/validation/spec/info.go Outdated Show resolved Hide resolved
@sttts
Copy link
Contributor

sttts commented Jul 14, 2023

@alexzielenski
Copy link
Contributor Author

removed string slice logic

@alexzielenski
Copy link
Contributor Author

/label tide/merge-method-squash

@k8s-ci-robot k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Jul 14, 2023
@sttts
Copy link
Contributor

sttts commented Jul 14, 2023

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 14, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alexzielenski, apelisse, jpbetz, sttts

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 14, 2023
@k8s-ci-robot k8s-ci-robot merged commit 0653797 into kubernetes:master Jul 14, 2023
@alexzielenski alexzielenski mentioned this pull request Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants