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.

Sign up for GitHub

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

Jump to bottom

Feature implementation: Validation rules for Custom Resource Definitions using the CEL expression language #106051

Merged
merged 5 commits into from Nov 16, 2021
Merged

Feature implementation: Validation rules for Custom Resource Definitions using the CEL expression language #106051

merged 5 commits into from Nov 16, 2021

Conversation

jpbetz
Copy link
Contributor

@jpbetz jpbetz commented Nov 1, 2021 •
edited by liggitt

Authors: @cici37, @jpbetz

What type of PR is this?

/kind feature /kind api-change

What this PR does / why we need it:

Add x-kubernetes-validator to CRD for expression validation.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Validation rules for Custom Resource Definitions can be written in the [CEL expression language](https://github.com/google/cel-spec) using the `x-kubernetes-validations` extension in OpenAPIv3 schemas (alpha). This is gated by the alpha "CustomResourceValidationExpressions" feature gate.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

[KEP]: <https://github.com/kubernetes/enhancements/tree/master/keps/sig-api-machinery/2876-crd-validation-expression-language>

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Nov 1, 2021
@k8s-ci-robot k8s-ci-robot requested review from adtac, andrewsykim and a team November 1, 2021 17:19
@k8s-ci-robot k8s-ci-robot added area/apiserver area/cloudprovider area/code-generation area/dependency Issues or PRs related to dependency changes area/kubectl sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/auth Categorizes an issue or PR as relevant to SIG Auth. sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/storage Categorizes an issue or PR as relevant to SIG Storage. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Nov 1, 2021
@jpbetz jpbetz changed the title WIP - Move CEL integration code from kube-openapi to k/k and expand test coverage Move CEL integration code from kube-openapi to k/k and expand test coverage Nov 1, 2021
@jpbetz jpbetz force-pushed the cel-port branch 2 times, most recently from c36bccd to b243523 Compare November 15, 2021 18:36
@jpbetz jpbetz mentioned this pull request Nov 15, 2021
Closed
@liggitt
Copy link
Member

liggitt commented Nov 15, 2021 •
edited

Done reviewing, looks like we're in the home stretch:

one bug to fix before merge:

two follow-up issues to open/track before beta:

The test coverage here is excellent, thanks for diligently adding that as you went

This was referenced Nov 16, 2021
Closed
Closed
@jpbetz
Copy link
Contributor Author

jpbetz commented Nov 16, 2021

one bug to fix before merge:

Pushed a commit to address this. (There is also some API doc grammar fixes that are quite minor).

two follow-up issues to open/track before beta:

Tracking issue: #106440

Tracking issue: #106438

(both issues are being added to the KEP graduation criteria via kubernetes/enhancements#3039)

Huge thanks for the review @liggitt!

@jpbetz
Copy link
Contributor Author

jpbetz commented Nov 16, 2021

Rebased and squashed.

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Nov 16, 2021
@liggitt
Copy link
Member

liggitt commented Nov 16, 2021

/lgtm
/approve

@liggitt liggitt moved this from In progress to API review completed, 1.23 in API Reviews Nov 16, 2021
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 16, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jpbetz, liggitt

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 Nov 16, 2021
@jpbetz
Copy link
Contributor Author

jpbetz commented Nov 16, 2021

/retest

@k8s-ci-robot k8s-ci-robot merged commit c84da4e into kubernetes:master Nov 16, 2021
SIG Node CI/Test Board automation moved this from Archive-it to Done Nov 16, 2021
SIG Auth Old automation moved this from Needs Triage to Closed / Done Nov 16, 2021
This was referenced Nov 16, 2021
Closed
Closed
@cici37 cici37 mentioned this pull request Nov 24, 2021
Merged
@liggitt liggitt mentioned this pull request Feb 4, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TristonianJones TristonianJones TristonianJones left review comments

@sttts sttts sttts left review comments

@liggitt liggitt liggitt left review comments

@alculquicondor alculquicondor alculquicondor left review comments

@cici37 cici37 cici37 left review comments

@dgrisonnet dgrisonnet dgrisonnet left review comments

@adtac adtac Awaiting requested review from adtac

@andrewsykim andrewsykim Awaiting requested review from andrewsykim

Assignees

@liggitt liggitt

@alculquicondor alculquicondor

Labels
api-review Categorizes an issue or PR as actively needing an API review. approved Indicates a PR has been approved by an approver from all required OWNERS files. area/apiserver area/cloudprovider area/code-generation area/dependency Issues or PRs related to dependency changes area/kubectl area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/auth Categorizes an issue or PR as relevant to SIG Auth. sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/storage Categorizes an issue or PR as relevant to SIG Storage. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
API Reviews
Status: API review completed, 1.23
SIG Auth
Archived in project
SIG Node CI/Test Board
Done
SIG Node CI/Test Board
Archived in project
SIG Node PR Triage
Triage
SIG Auth Old
Closed / Done
Milestone
v1.23
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@jpbetz @k8s-triage-robot @alculquicondor @cici37 @sttts @liggitt @k8s-ci-robot @TristonianJones @dgrisonnet